Dev/composability solidity libraries

[maxpolizzo]

This PR contains the following:

• Solidity libraries which make it possible to easily implement secure interactions with Solana's System and SPL Token programs.
• TestComposability contract implementing example use cases for those libraries along with user authentication
• Demonstration scripts implementing example interactions with the TestComposability contract

To do:

• Change Solidity libraries functions visibility to internal (to use as embedded libraries)
• Refactor demo scripts logs (don’t log deployer at each script, etc)
• Remove calls to TestComposability.ata() from demo scripts, replace with calls to TestComposability.getAssociatedTokenAccount()
• Add seeds and nonces to config file
• Have identical accounts names across all demo scripts: deployer, sender, user, …

Run with: npx hardhat run ./scripts/TestComposability/test.js --network curvestand

[mnedelchev-vn]

@maxpolizzo a question for my own understanding - why did you leave the library methods as public? Is this on purpose or a mistake? Leaving them as internal will not deploy the libraries as individual contracts and it will include the library methods to the codebase of the contract which uses the libraries, but using public visibility will deploy each one of the libraries and requests to the library will be a bit more expensive, because they will be external calls.

[maxpolizzo]

@maxpolizzo a question for my own understanding - why did you leave the library methods as public? Is this on purpose or a mistake? Leaving them as internal will not deploy the libraries as individual contracts and it will include the library methods to the codebase of the contract which uses the libraries, but using public visibility will deploy each one of the libraries and requests to the library will be a bit more expensive, because they will be external calls.

I did so assuming that these libraries would be deployed once and for all and used as "linked libraries" by other contracts. Maybe it is not the case, they could also be used as "embedded libraries" in which case their functions should be internal.

[mnedelchev-vn]

@maxpolizzo a question for my own understanding - why did you leave the library methods as public? Is this on purpose or a mistake? Leaving them as internal will not deploy the libraries as individual contracts and it will include the library methods to the codebase of the contract which uses the libraries, but using public visibility will deploy each one of the libraries and requests to the library will be a bit more expensive, because they will be external calls.

I did so assuming that these libraries would be deployed once and for all and used as "linked libraries" by other contracts. Maybe it is not the case, they could also be used as "embedded libraries" in which case their functions should be internal.

For the initial version let's keep them embedded

[mnedelchev-vn]

For better readable code we can change this to bytes32(0)

[mnedelchev-vn]

This library is not needed as we already have this functionality here. Loops always require a bit heavier gas cost execution, here is quick report of comparing the 2 approaches:

function convertUintToLittleEndianBytes32(uint256 bigEndian) public pure returns (bytes32 littleEndian) {
        assembly {
            for {let i := 0} lt(i, 32) {i := add(i, 1)} {
                let nextBEByte := byte(sub(31, i), bigEndian) // Get BE bytes starting from the right
                let nextLEByte := shl(sub(248, mul(i, 8)), nextBEByte) // Shift left to get LE bytes
                littleEndian := add(littleEndian, nextLEByte) // Add LE bytes to return value
            }
        }
    }

    // execution cost - 3515 gas
    function toBytes8(uint256 input) public pure returns (bytes8) {
        return bytes8(convertUintToLittleEndianBytes32(input));
    }

    function readLittleEndianSigned256(uint256 input) public pure returns (int256) {
        input = ((input << 8) & 0xFF00FF00FF00FF00FF00FF00FF00FF00) | ((input >> 8) & 0x00FF00FF00FF00FF00FF00FF00FF00FF);
        input = ((input << 16) & 0xFFFF0000FFFF0000FFFF0000FFFF0000) | ((input >> 16) & 0x0000FFFF0000FFFF0000FFFF0000FFFF);
        input = ((input << 32) & 0xFFFFFFFF00000000FFFFFFFF00000000) | ((input >> 32) & 0x00000000FFFFFFFF00000000FFFFFFFF);
        input = ((input << 64) & 0xFFFFFFFFFFFFFFFF0000000000000000FFFFFFFFFFFFFFFF0000000000000000) | ((input >> 64) & 0x0000000000000000FFFFFFFFFFFFFFFF0000000000000000FFFFFFFFFFFFFFFF);
        return int256((input << 128) | ((input >> 128) & 0xFFFFFFFFFFFFFFFF));
    }

    function _uint256ToBytes8(uint256 input) internal pure returns(bytes8) {
        bytes8 result;
        int256 value = readLittleEndianSigned256(input);
        assembly {
            result := value // Implicit truncation to the lower 8 bytes
        }
        return result;
    }

    // execution cost - 624 gas
    function toBytes8Optimized(uint256 input) public pure returns (bytes8) {
        return _uint256ToBytes8(input);
    }

Method toBytes8 takes 3515 gas meanwhile toBytes8Optimized takes nearly 5 times lesser gas usage.

[mnedelchev-vn]

Here you have the same logic, I advise you to add it into an internal method and call it here and inside the public method getCreateWithSeedAccount

[mnedelchev-vn]

The ATA calculation can be moved inside LibSPLTokenProgram.formatRevokeInstruction and this method can accept the_tokenMint parameter instead the userATA. The idea behind this is to abstract away as much as possible Solana logic from the EVM developer.

[mnedelchev-vn]

Also I'm confused why do you need the ATA nonce at all? Here is quick way how to get an user's ATA on-chain:

function getAssociateTokenAccount(bytes32 solanaAccount, bytes32 tokenMint) public view returns (bytes32) {
        return CALL_SOLANA.getSolanaPDA(
            ASSOCIATED_TOKEN_PROGRAM,
            abi.encodePacked(
                solanaAccount,
                TOKEN_PROGRAM,
                tokenMint
            )
        );
    }

[mnedelchev-vn]

I think it's also good idea to include this getter inside LibSPLTokenProgram.sol

[maxpolizzo]

The getAssociatedTokenAccount function implemented in TestComposability contract and getSolanaPDA function from the CALL_SOLANA precompile don’t return the same pubKey. 



For instance:

3iSkPAZeSaKQLy3TzVec3VNsc8zc8yvJ5XvJm5bArAGo <-- ATA derived by getAssociatedTokenAccount function with nonce == 0
ApeW2S6fEjfcHF8KLjaK6NsRrdRTKsMJ7QHDhXbpSHEi <-- PDA derived from the same account by getSolanaPDA function

The nonce is a “bump seed” used to ensure that PDA falls off-curve. See here.

Here we added a note saying that getSolanaPDA uses the same logic as findProgramAddressSeed implementation in @solana/web3.js npm module but findProgramAddressSeed does return the nonce used to derive the ATA, while getSolanaPDA does not.
I guess getSolanaPDA still iterates a nonce under the hood to derive an off-curve PDA, but it doesn't give the option to use different nonces.

The reason why I left the option to choose the nonce is that
in the specific case of ATAs the nonce can be used to generate several different ATAs for the same account . This could be useful depending on the use case (for instance when using bots or when managing many accounts belonging to different users, having different ATAs allows to easily keep track of balances). If we don't care about this then we could use getSolanaPDA function.

[mnedelchev-vn]

ok I understand your point, imo this logic with querying ATAs by nonces is not needed now, but it could be needed on a later stage for some integration. I'd suggest make one private method in the LibSPLTokenProgram.sol and 2 internal methods that point to it:

1. first internal method does pretty much the same thing as getAssociatedTokenAccount ( under the hood querying ATA with nonce 0, but not bothering the user to worry about nonce )
2. second internal method is to query ATA by given nonce parameter

But please move both of these methods inside LibSPLTokenProgram.sol and TestComposability.sol should be calling them.

[maxpolizzo]

Added the following functions to LibSPLTokenProgram library:

function getAssociatedTokenAccount(
    bytes32 _tokenMint,
    bytes32 userPubKey
) internal view returns(bytes32) {
    // Returns ATA derived with  nonce == 0 by default
    return _getAssociatedTokenAccount(_tokenMint, userPubKey, 0);
}

function getAssociatedTokenAccount(
    bytes32 _tokenMint,
    bytes32 userPubKey,
    uint8 nonce
) internal view returns(bytes32) {
    return _getAssociatedTokenAccount(_tokenMint, userPubKey, nonce);
}

function _getAssociatedTokenAccount(
    bytes32 _tokenMint,
    bytes32 userPubKey,
    uint8 nonce
) private view returns(bytes32) {
    return CALL_SOLANA.getResourceAddress(sha256(abi.encodePacked(
        userPubKey,
        LibSPLTokenProgram.TOKEN_PROGRAM_ID,
        _tokenMint,
        nonce,
        LibSPLTokenProgram.ASSOCIATED_TOKEN_PROGRAM_ID
    )));
}

And the following function to TestComposability contract:

function getAssociatedTokenAccount(
    bytes32 _tokenMint,
    bytes32 userPubKey
) public view returns(bytes32) {
    // Returns ATA derived with nonce == 0 by default
    return LibSPLTokenProgram.getAssociatedTokenAccount(_tokenMint, userPubKey);
}

Regarding the default nonce value, here we use 0 but the @solana/spl-token NPM library starts with 255 and iterates down until it finds an ATA that is off-curve (this is the canonical way of deriving ATAs). So I'm wondering if we shouldn't use 255 as the default value maybe?

[mnedelchev-vn]

_tokenMint can be moved inside LibSPLTokenProgram.formatMintToInstruction. Again the idea behind this is to not bother too much the EVM dev with Solana stuff

[maxpolizzo]

The specification for those libraries was to safely format instructions for a specific Solana program, but ATA and token mint derivations involve some authentication logic when deriving the salt value passed to CALL_SOLANA's getResourceAddress
function.

In the TestComposability example this authentication uses msg.sender and a seed (for token mint derivation) or a nonce (for ata derivation).

If we move the salt derivation inside the library then devs using the library won't have a choice on the authentication logic: maybe they would like to use different parameters to derive the salt (based on their use case).

Maybe they would like to use tx.origin instead of msg.sender (if they are using another intermediary contract to call the library method for instance), or maybe they would like to have an additional seed involved in the salt calculation.

If we add too much logic into a single library we lose modularity, which increases the likelihood that devs will re-write their own library to fit their use case. It would also be possible to have another library for authentication on top of the library that formats instructions, which would keep each library modular.

What do you think?

[mnedelchev-vn]

Well argumented, I agree with you here. Please just make it clear through comments that it's entirely possible to change the _tokenMint calculation into whatever they desire. For example it could be something else rather than using msg.sender or tx.origin - could be anything.

[maxpolizzo]

Added the following function to TestComposability contract:

function getTokenMintAccount(bytes memory seed) public view returns(bytes32) {
    // Returns the token mint account derived from from msg.sender and seed
    return CALL_SOLANA.getResourceAddress(sha256(abi.encodePacked(
        msg.sender, // msg.sender is included here for authentication
        seed // Seed that has been used to create token mint
    )));
}

And the following comments to testCreateInitializeTokenMint function in the same contract:

// Create SPL token mint account: msg.sender and a seed are used to calculate the salt used to derive the token
// mint account, allowing for future authentication when interacting with this token mint. Note that it is
// entirely possible to calculate the salt in a different manner and to use a different approach for
// authentication
tokenMint = CALL_SOLANA.createResource(
    sha256(abi.encodePacked(
        msg.sender, // msg.sender is included here for future authentication
        seed // using different seeds allows msg.sender to create different token mint accounts
    )), // salt
    LibSPLTokenProgram.MINT_SIZE, // space
    LibSPLTokenProgram.MINT_RENT_EXEMPT_BALANCE, // lamports
    LibSPLTokenProgram.TOKEN_PROGRAM_ID // Owner must be SPL Token program
);

[mnedelchev-vn]

Pls move _tokenMint inside LibSPLTokenProgram.formatUpdateMintAuthorityInstruction.

[maxpolizzo]

Addressed here

[mnedelchev-vn]

Pls move senderATA inside LibSPLTokenProgram.formatTransferInstruction

[maxpolizzo]

Addressed here

[mnedelchev-vn]

Pls move ata inside LibSPLTokenProgram.formatInitializeAccount2Instruction

[maxpolizzo]

Addressed here

[mnedelchev-vn]

Pls move tokenMint inside LibSPLTokenProgram.formatInitializeMint2Instruction

[maxpolizzo]

Addressed here

[mnedelchev-vn]

This method can be removed, in another comment I did share ATA account calculation without the need of passing nonce.

[maxpolizzo]

Addressed here