solana-labs / solana-program-library Goto Github PK

The native_mint id, like all other mints, should always be changed whenever the spl-token program id is changed. RPC, in particular, makes the valid assumption that this is the case.

A human solution: document and remember to change it
A programatic solution: change the last ~32 digits of the native_mint id to be the same as the spl-token program id

The NewToken instruction expects a destination account that accepts the newly minted tokens. But for mintable tokens (non-fixed supply) the initial supply could be zero with the expectation that tokens will be minted later via MintTo. In that case, no destination account is necessary.

Proposed changes:

• Change the NewToken instruction to not expect a destination account if supply is zero
• If the initial supply is zero the owner account will be mandatory.

Token delegation is one way that an account owner can give authority to a program to issue transfer instructions.

Token account delegation requires three steps

• Allocate a new account via 'System' program
• Issue TokenInstruction::NewAccount to create the delegate token account
• issue 'TokenInstruction::Approve' to approve the delegate

Using a delegate requires both the delegate and the original source account to be present when performing a transfer. This gets especially painful for programs like token-swap that perform transfers from multiple accounts

Delegation accounts also have a typically short lifespan and there is not defund instruction to reclaim any lamports held in them. If they are included in the rent exemption enforcement being proposed there should probably be a defund instruction, though that adds another step to the delegation process.

Delegation could be reduced to a simple step and single account if the token account itself held one delegation public key and amount. If all of the amount is transferred delegation ends or if a new delegation instruction is issued it would replace any existing delegation. Canceling delegation could also occur by specifying the primary owner in a delegate instruction.

Single-step delegation would also be more conducive to transactions performing an atomic delegation and operation transaction.

Problem

InitializeAccount doesn't check if mint_info is valid (exists, initialized, and owned by token program) and so tokens can be created with invalid mints. These token accounts are useless because without a valid mint, they can never hold a token balance.

Suggested Changes

• For developer experience, we should fail fast on token creation to avoid later confusion when token accounts are attempted to be minted to.
• Consider forcing mint accounts rent exempt so that token accounts are always valid

Once solana-labs/solana#11745 lands and ships in a Solana 1.3 release, restore the token-swap tests:

Problem

Solana distinguishes programs from smart contracts, but the SPL doesn't contain an example of that.

Proposed Solution

Add Budget from the Solana repo.

The user of a token program, such as a generic wallet or the spl-token command-line, doesn't generically have trusted way to determine the correct decimals value for a given token. RPC will tell it, but maybe RPC is lying. The user might know and can provide it, but maybe they are mistaken.

As a safety/sanity check, if the decimals are provided as input into each instruction alongside the amount, the program itself can confirm the decimals are correct before performing the operation.

Problem

Token's are a great way to implement a voting system. For the system to work we need a "hold" time component as well.

Solution

Add a instruction that sets the current slot

• InitializeHODLSlot

Any withdrawals reset the HODLSlot to None.

A vote program can then generate a ballot from a checkpoint of the tokens balance, and the HODLSlot. To Vote the same token must be presented with the ballot with its HODLSlot set to the expected value.

tag: @jackcmay @CriesofCarrots

Problem

This actually contains two problems with one solution. When a new token-swap is initialized between token A and B, we assume that the two token accounts have the same quantity without doing a check: https://github.com/solana-labs/solana-program-library/blob/master/token-swap/program/src/processor.rs#L195

Solution

The simplest solution would be to add a check to make sure that token_a.amount == token_b.amount and keep the code as is, but even better would be to use the geometric mean of the two provided accounts, ie. sqrt(token_a.amount * token_b.amount). This is recommended by Uniswap v2 in the whitepaper https://uniswap.org/whitepaper.pdf section 3.4, to protect the intializer of the swap in the case where the initial provided amounts are off the market.

Currently, the total supply of tokens is created upon the NewToken instruction. But there are some tokens that require an unlimited supply and the ability to mint new tokens at any time (BRRR).

One suggestion is that at token creation time, an admin key is stored in the token account. Only the admin key can then invoke a Mint instruction to create new supply, perhaps with another instruction to replace the admin key

Problem

People creating currencies with Token will want Ledger Wallet support. To get there, the Ledger Wallet app will need to parse Token instructions. It's easiest in C, though Rust is technically possible. Here's how we parse System, Stake, etc:

https://github.com/solana-labs/ledger-app-solana/tree/master/libsol

Proposed Solution

Add an equivalent to libsol for Token instructions.

cc: @jackcmay @t-nelson

Problem

A couple dependabot PR's on this repo is all it takes to get the solana CI queue backed up to 50+ tasks

Proposed Solution

Switch to Github actions

We lost the total supply field in the mint when we realized that if token accounts aren't created rent-exempt, they could just disappear and cause the total supply to be inflated.

However we've failed to discover a compelling reason to permit non-rent-exempt token accounts. Additionally with the new CloseAccount instruction, you can reclaim your SOL from a token account at any time.

Requiring rent exemption also simplifies wallets and other token clients, as they no longer need to be considered about a token account disappearing because it was underfunded, either by accident or maliciously.

Problem

A bad client might issue the "create account" and "init mint" in separate transactions. An attacker could front run and initialize the mint with themselves as owner and the original creator might not realize this. Then, the attacker could mint tokens whenever they please

This is applies to init multisig and init token account as well

Suggested Fix

Reject init instructions if initialized account is not a signer

https://travis-ci.org/github/solana-labs/solana-program-library/builds/694862433

Run test: loadTokenProgram
Connection to cluster established: http://devnet.solana.com { 'solana-core': '1.2.1 56e8319a' }
Loading Token program...
Error: failed to send transaction: Transaction simulation failed: TransactionError::BlockhashNotFound
    at Connection._callee31$ (/home/travis/build/solana-labs/solana-program-library/token/node_modules/@solana/web3.js/src/connection.js:1437:13)
    at tryCatch (/home/travis/build/solana-labs/solana-program-library/token/node_modules/@babel/runtime/node_modules/regenerator-runtime/runtime.js:45:40)
    at Generator.invoke [as _invoke] (/home/travis/build/solana-labs/solana-program-library/token/node_modules/@babel/runtime/node_modules/regenerator-runtime/runtime.js:271:22)
    at Generator.prototype.<computed> [as next] (/home/travis/build/solana-labs/solana-program-library/token/node_modules/@babel/runtime/node_modules/regenerator-runtime/runtime.js:97:21)
    at asyncGeneratorStep (/home/travis/build/solana-labs/solana-program-library/token/node_modules/@babel/runtime/helpers/asyncToGenerator.js:3:24)
    at _next (/home/travis/build/solana-labs/solana-program-library/token/node_modules/@babel/runtime/helpers/asyncToGenerator.js:25:9)
    at processTicksAndRejections (internal/process/task_queues.js:97:5)
The command "npm run start --prefix token" exited with 0.

Dependabot couldn't find a package.json for this project.

Dependabot requires a package.json to evaluate your project's current JavaScript dependencies. It had expected to find one at the path: /token/package.json.

If this isn't a JavaScript project, or if it is a library, you may wish to disable updates for it from within Dependabot.

View the update logs.

Could use a trait object:

    pub fn token_transfer(                                                                            
        &self,
        accounts: &[AccountInfo],                                                                     
        token: &Pubkey,
        destination: &Pubkey,                                                                         
        amount: u64,
    ) -> Result<(), ProgramError> {
        let swap_string = self.self_pubkey.to_string();                                               
        let signers = &[&[&swap_string[..32]][..]];                                                   
        let ix = spl_token::instruction::transfer(                                                    
            self.token_program,                                                                       
            self.authority,                                                                           
            token,
            destination,                                                                              
            amount,                                                                                   
        )?;
        invoke_signed(&ix, accounts, signers)                                                         
    } 

Problem

When users deposit / withdraw liquidity they have no assurance that the ratio they see in the UI is the same as what's executed on-chain

Proposed Change

TBD

Replace the token1 usage in master/v1.3/v1.2 with token2. We don't need to retain backwards compatibility with token1

The use of unsafe typecasting in instruction pack/unpack significantly reduces the instruction count over alternatives but is also scary to look at and is more likely it contain bugs (or have bugs accidentally introduced). Further this is not a general pattern we want to see in all Solana contracts.

Consider alternatives that either make the unsafe go away entire, or pushes the unsafe into macros or generic libraries where the possibility of human error is reduced.

Because targetDir is defined outside perform_action, it keeps the positional argument $2 from the original command. In the case of ./do.sh build all, this is all. Therefore this check fails:

Current spl-token is completed rent unaware, which leads to a couple problems:

1. If the Token account is not rent-exempt,

   solana-program-library/token/src/state.rs

   Lines 19 to 26 in f8f51c1

   	pub struct Token {
   	/// The total supply of tokens.
   	pub info: TokenInfo,
   	/// Optional token owner, used to mint new tokens. The owner may only
   	/// be provided during token creation. If no owner is present then the token
   	/// has a fixed supply and no further tokens may be minted.
   	pub owner: COption<Pubkey>,
   	}

   and if it's rent collected then all the basic information about the token is lost, as well as the ability to mint new tokens or burn existing tokens
2. If the Account account is not rent-exempt,

   solana-program-library/token/src/state.rs

   Lines 41 to 52 in f8f51c1

   	pub struct Account {
   	/// The type of token this account holds.
   	pub token: Pubkey,
   	/// Owner of this account.
   	pub owner: Pubkey,
   	/// Amount of tokens this account holds.
   	pub amount: u64,
   	/// If `delegate` is None, `amount` belongs to this account.
   	/// If `delegate` is Option<_>, `amount` represents the remaining allowance
   	/// of tokens this delegate is authorized to transfer from the `source` account.
   	pub delegate: COption<AccountDelegate>,
   	}

   and if it's rent collected then the supply field in the Token account is now wrong.

(1) is probably not a big issue, but (2) certainly is.

Possible solution:
a) Require the Token account to be rent-exempt
b) Remove the supply field from Token account so that an Account account that is not rent-exempt cannot poison the supply. This increases the burden on Rpc and other clients, as the token supply must now be calculated by enumerating all the Account accounts for a given token

Running the following in a loop produces different versions of token.h occasionally:

rm token/program/inc/token.h
SPL_CBINDGEN=1 ./do.sh build-lib token -D warnings

The ci check to catch header changes has been disabled: #262

Reisen from Chrous One provided the following feedback (continued from https://github.com/solana-labs/solana-com/issues/390)

7. Opening both of these have decent README's but include a lot of web stuff, where I only want to build a contract.
8. Looking at build:bpf-rust to see how I can create a new project with JUST the BPF compilation step lands me here: https://github.com/solana-labs/example-tictactoe/tree/master/program-bpf-rust
9. I see do.sh, realise I need node/npm installed, and that this script runs a script located under node_modules.
10. I didn't want to dig further.

At this point I just cd'd into the C BPF folder and ran make and embraced the darkness of C.

The only dependency required to build the program is the bpf-sdk which node currently downloads, we could obtain the bpf-sdk other ways and give developers a non-node path to program development. For end-to-end testing/development, the client is still based on Solana-web3.js which requires node/js

Sometimes users want to burn some tokens. Let's add the token program equivalent of 1nc1nerator11111111111111111111111111111111

Problem

It's not easy to setup SPL token accounts in a test environment or in a UI that creates manages tokens because there is no published library

Proposed Solution

Deploy @solana/spl-token as is for consumption

tag @jackcmay

Problem

Accounts are determined to be multisig if their data size equals the size of a multisig account. There is no restriction on non-multisig accounts that they must not equal the size of a multisig account so other token account types could be mistaken as a multisig account if they are big enough and are set as a token account owner

Doesn't seem likely that this would happen in practice, but if it did, it could result in an owner account that has always passes the owner validation check if m is 0.

Proposed Change

• Assert that token account types are different sizes
• Require token accounts to have a specific size

Problem

The transfer method accepts authority as a public key or an account. If it is an account, it does not sign with all accounts in multiSigners

Proposed Solution

• Only accept authority as an account and use multiSigners consistently

Clients dealing with a token generically need to find metadata about that token: name, icon, web presence of the issuer, etc. The various ERC token standards have a lot to say about this, we can probably reuse most/all of those specs

However in the token2 timeframe, we don't need to actually define what the metadata is. This can come later. For token2, we just need a reserve place in the Mint to hold the details about how to locate the metadata.

Proposal:

1. Add the field metadata_authority: COption<Pubkey> to the end of the Mint account
2. New metadata_authority parameter to the InitializeMint instruction
3. New AuthorityType for metadata
4. Support for changing the metadata address through SetAuthority

A couple possible metadata implementations (to be fully designed later):
a) The metadata_authority points to an account with inline CBOR data describing the token's metadata
b) The metadata_authority points to an account with an https/ipfs URL to the token's metadata

Problem

Users don't have any guarantee that the token ratio they submit a swap at will be the same as what is executed on-chain

Proposed Changes

• Add a min output to swap instruction

ERC20 uses 256 bits right now, so spl-token can't map 1:1 to ERC20. This seems like a general usability problem of spl-token.

Along the way let's make 18 decimals the (strongly recommended) default for spl-token

Problem

With a proliferation of SPL tokens, creating all possible cross-pairs on token-swap could mean liquidity is very spread out, creating arbitrage opportunities.

Solution

A simple solution, and the one used by Uniswap v1, is to have every pair include the native token, e.g. A/B is traded by going through A/SOL and B/SOL. The negative aspect of this is a higher cost of trading. This issue has been submitted to start the discussion.

Problem

The separate Cargo.lock per program will mean we can't share lib dependencies. For example, if the Token program wants to include a Memo, it should add spl-memo as a dependency, but if it does that while there's a Cargo.lock file, all those dependencies will need to match perfectly.

Proposed solution

Add a top-level Cargo.toml and Cargo.lock, delete the nested ones

1. Remove amount: u64 argument. Instead the user can issue a MintTo instruction immediately after InitializeMint. This allows the removal of the first account key
2. Remove the second account key. The supply can be fixed by issuing a SetAuthority(AuthorityType::MintTokens, None) instruction after MintTo

These changes have no affect on expressibility and will reduce a number of branches from InitializeMint

There are cases when the SOL required for rent at the creation of an SPL token account will be subsidized by a 3rd party.

However upon assigning ownership of the token account to the targeted user, the subsidizer loses control of their rent payment. This opens the door for a financial attack where users request token accounts from the subsidizer only to immediately close them in order to drain SOL from the subsidizer:

The ability of the subsidizer to reclaim the SOL would also incentivize them to "garbage collect" abandoned token accounts of 0 balance.

Adding a new CloseAccount authority to a token account would resolve the above concerns: close_authority: Option<Pubkey>. If close_authority is None (default), the token account holder may close the account (current behaviour). However the SetAuthority instruction (going in via #297) could be used to change the CloseAccount authority to any other address.

Problem

Hand-written serializers/deserializers:
https://github.com/solana-labs/solana-program-library/blob/master/token/src/state.rs#L524

Proposed Solution

Pick a serialization library and switch to it before we publish the Token program to Mainnet Beta. There's plenty of precedent for Serde/Bincode, but I'm open to something more language agnostic.

Dependabot couldn't find a package.json for this project.

Dependabot requires a package.json to evaluate your project's current JavaScript dependencies. It had expected to find one at the path: /package.json.

If this isn't a JavaScript project, or if it is a library, you may wish to disable updates for it in the .github/dependabot.yml file in this repo.

View the update logs.

The current program source layout is inadequate for working on newer version of a SPL program while maintaining the existing deployed versions.

Proposal is to refactor up the current token/ directory into:

token-1.0/  <-- represents the 1.0.0 token that has already been deployed to mainnet-beta
token-dev/ <-- the 1.1.0 token that will be deployed sometime in the future.

When token-dev/ is ready to be deployed, it's copied into token-1.1/ and then token-dev/ moves up to "1.2.0".

The benefit of this approach is a clear representation of the on-chain source code for a given token version while retaining the ability to ship new patch releases for each token crate. Of course since token 1.0.0 program is deployed, it can't be changed but the client portion of the 1.0.0 token may still need to change from time to time (such as if we ship a breaking change in the solana-sdk crate).

The same model applies to the other SPL programs. For token-swap there would only be a token-swap-dev since 1.0.0 has not shipped yet.

Downstream users would like to be able to ultilize multisignature on Token Accounts. A Multisig program would be an option down the road, when cross-program invocations are enabled.
In the meantime, the shortest path seems to be to add M-of-N multisig support in Token.

Instead of just one account owner, enable configuration of each Token account to have N authorities (1 <= N <= 11) and M required signers for transfers and updates to authorities and num-required-signers.

Problem

In the js client and processor, when creating and validating the program address to be used by the swap account, we only include the swap account's public key as a seed. This could create a situation in which we have multiple swap accounts and pool tokens to cover swaps from A to B, which would create arbitrage opportunities, along with a confusing user experience.

Solution

Include the token pubkeys as seeds in create_program_address. Also, the token pubkeys should be sorted deterministically (perhaps just alphabetically) in order to avoid having two separate swap accounts for A/B and B/A. I considered also including the pool token as a seed, but that wouldn't solve the problem, since anyone could just mint a new pool token and create a new token swap account for an existing pair.

spl-token currently only supports fungible tokens. With some minor tweaks it can also support non-fungible and semi-fungible tokens.

Proposed changes:

1. Each Token Account has an additional field: item: u64
2. Transfer instruction now only permits the transfer between (a) source and destination token accounts with the same item, or (b) a destination token account with an amount of 0, which point the destination item is overwritten by the source token account's item
3. Approve/Revoke/SetOwner/CloseAccount are item agnostic
4. MintTo is expanded to include an item field. The destination account to mint into must either (a) have the same item field or (b) an amount of 0, which point the destination item is overwritten by the MintTo's item field

Note: this description assumes #308 has been implemented.

Fungible Minting

Use existing workflow, always MintTo with an item of 0 (by convention all fungible tokens should not mint tokens with an item other than 0)

Non-Fungible Minting

1. InitializeMint { decimals: 0, .. } <-- init the collection (no fractional items)
2. MintTo { amount: 1, item: 0 } <-- create the first item
3. MintTo { amount: 1, item: 1 } <-- create the second item
4. MintTo { amount: 1, item: 2 } <-- create the third item
5. SetAuthority(AuthorityType::MintTokens, None) <-- no more items may be minted

A separate MintTo instruction for each item feels heavy handed, but I don't see a better way. The associated SystemInstruction::CreateAccount and InitializeAccount instructions are not shown but are also required before each MintTo

To send one NFT to another party:

1. SetOwner the token account to them
2. Transfer into their token account. Destination account which must be initialized with an amount of 0

Semi-Fungible Minting

1. InitializeMint { decimals: 0, .. } <-- init the collection (no fractional items)
2. MintTo { amount: 10, item: 0 } <-- create 10 of the first item
3. MintTo { amount: 1, item: 1 } <-- create 1 of the second item
4. MintTo { amount: 100, item: 2 } <-- create 100 of the third item
5. SetAuthority(AuthorityType::MintTokens, None) <-- no more items may be minted

To send Semi-Fungible Tokens to another party:

1. SetOwner the token account to them, which will transfer the full amount of your item.
2. Transfer one or more of your items into their token account. Destination account which must be initialized with an amount of 0, or already hold one or more items of the same item.

It's useful to transfer funds to yourself during testing. We added this support recently to system transfers, and should do the same for spl-token.

Example failure:

$ spl-token transfer 5jYj95QRJt5KSvS3naU7vMDnaNT2aXzacyY2waRdJwCg 1 5jYj95QRJt5KSvS3naU7vMDnaNT2aXzacyY2waRdJwCg
Transfer 1 tokens
  Sender: 5jYj95QRJt5KSvS3naU7vMDnaNT2aXzacyY2waRdJwCg
  Recipient: 5jYj95QRJt5KSvS3naU7vMDnaNT2aXzacyY2waRdJwCg
rpc request error: RPC Error response: {"code":-32002,"message":"Transaction simulation failed: Error processing Instruction 0: custom program error: 0xb9f0002"}

The current user of program/ has agreed to upgrade to program2/, so we don't need to keep the old program/ around anymore.

We can save program2/ for the next token upgrade

Problem

The spl-token v1 program id is well known but not embedded inside the library. It should be exported for devs to use.

Dependabot couldn't find a package.json for this project.

Dependabot requires a package.json to evaluate your project's current JavaScript dependencies. It had expected to find one at the path: /package.json.

If this isn't a JavaScript project, or if it is a library, you may wish to disable updates for it from within Dependabot.

View the update logs.

An authority may want the ability to "freeze" specific accounts.

Add a new authority to the mint and a new flag in accounts. The authority is able to toggle that flag. Setting the flag to "frozen" disables the account owner and delegate's authority.

The API documentation for https://www.npmjs.com/package/@solana/spl-token lives only in the code right now. ESDoc has been working out ok for solana-web3.js, so we could continue to use that solution or look around for an alternative

The js commands in token-swap.sh are not successful:

Building the token program requires the program feature when building the Solana SDK, but the latest solana-bpf-loader-program depends on the full Solana SDK for things like Transaction and Signature. These dependencies were added while supporting cross-program invocations.

The Token program's benches were removed due to this conflict when the Token program upgraded to Solana SDK 1.2.

The token program's benches need to be added back