Add default-token-account program

[mvines]

The SPL Default Token Account program defines and implements conventions around locating and creating the default token account for a user wallet as discussed in #612

TODO:

• Flesh out tests, does the thing actually work? Can't tell yet cause we don't have nice way to unit test invoke_signed()
• Add full DTA support to spl-token-cli. This'll really clean up the command-line UX!

[joncinque]

Chiming in here with an idea @bartosz-lipinski and I were discussing. It would be really useful to have a system instruction which allows users to create accounts for a program address directly, performing allocate and assign like you're doing in one instruction. wdyt?

[mvines]

I see how a SystemInstruction::CreateProgramAccount would be useful!

One problem I see is that until we have an account realloc instruction, CreateProgramAccount would allow anybody to grief a program's address space. For example I CreateProgramAccount an account of the wrong size at your default token account address. Now I just blocked you from ever creating a functioning default token account.

[joncinque]

Yeah that's definitely a big drawback, may be best to leave this off until later then

[mvines]

Certainly something to think about in general, I did find it annoying how I had find a "workaround" for account funding here. The solution of prepending a system Transfer instruction doesn't feel natural at all

[joncinque]

It can also create a slicker experience for frontend dapps. We discussed this before regarding swap applications, but let's say I only want one swap contract for ETH / SOL, to get that swap contract, I need to do getProgramAccounts with some authority that I generate on the frontend, then sift through those. With CreateProgramAccount, I can immediately getting the account I want based on those seeds.

[CriesofCarrots]

Certainly something to think about in general, I did find it annoying how I had find a "workaround" for account funding here. The solution of prepending a system Transfer instruction doesn't feel natural at all

Can you not pass in the funding account as a writable signer in the instruction and invoke() the transfer? This suggests that would work, although I haven't traced the code to confirm: https://docs.solana.com/implemented-proposals/cross-program-invocation#instructions-that-require-privileges

[CriesofCarrots]

I think you can just use invoke() here

[mvines]

Yep. I didn't at the time because then I'd need to stub out invoke() like invoke_signed(). But I'm knee deep into overhauling the non-BPF syscalls in sdk/src and with my latest local code, moving back to invoke() just works!

[mvines]

Can you not pass in the funding account as a writable signer in the instruction and invoke() the transfer? This suggests that would work, although I haven't traced the code to confirm: https://docs.solana.com/implemented-proposals/cross-program-invocation#instructions-that-require-privileges

No. The funding account is a system account that I control, and the program can't act as a signer for it.

[CriesofCarrots]

The funding account is a system account that I control, and the program can't act as a signer for it.

Which is why that system account would need to be passed in as a writable signer. Quoting from the above link: "if the instruction the caller is processing contains a signer or writable account, then the caller can invoke an instruction that also contains that signer and/or writable account."
What am I missing?

[CriesofCarrots]

Can you not pass in the funding account as a writable signer in the instruction and invoke() the transfer?

I was dead curious about this, so I threw together a little test program to try it: CriesofCarrots@2509f9b
I can confirm that it works against a local node. The only weird bit is that you have to pass the system program account into the outer instruction. But I suspect you'll have to do that for your other system_program cpi calls as well.

[mvines]

Whoa! Awesome, that's a big win for usability. I'll give that go!

[mvines]

new program-test framework is coming along nicely.

Check out default-token-account/program/run-tests.sh in this PR.

$ cargo test # tests the BPF program if present, otherwise test with the program compiled for host
$ bpf=1 cargo test # force BPF program tests (better run `cargo build-bpf` beforehand)
$ bpf=0 cargo test # force program compiled for host

The tests themselves are BanksClient-based. But while setting up the test environment, you can inject program and account fixtures either in the test directly, or by putting them in tests/fixtures/ and they'll be loaded automatically.

For example, default-token-account/program/tests/fixtures/spl_token.so is the actual SPL token BPF binary that the tests load and execute via CPI calls from default-token-account.

[mvines]

Demo of how solana-labs/solana#13245 is employed in a solana-program-test-based test harness to reduce the BPF compute budget

[mvines]

Tests are all passing using spl_default_token_account.so. Final blocker before landing this: adding CPI support for default-token-account when built as a native program

[jstarry]

From your guide, I was confused by:

Doing so will cause the DefaultTokenAccountInstruction::Exists instruction to fail

and thought that

Doing so will cause the DefaultTokenAccountInstruction::Assert instruction to fail

would be easier to understand

Suggested change

	Exists,
	Assert,

[mvines]

Ah, sure

[jstarry]

What if this was "Derived Token" program instead with an input seed? No input seed would be the same as your current implementation. Not sure if it needs to be generalized, but it could be nice to have an early convention for this.

[mvines]

What if this was "Derived Token" program instead with an input seed? No input seed would be the same as your current implementation. Not sure if it needs to be generalized, but it could be nice to have an early convention for this.

hmm, maybe! How about the DefaultDerivedAccount program, or just DefaultAccount? The Create instruction

    ///   0.   `[writeable]` Default account address derived from `get_default_account_address()`
    ///   1.   `[writeable,signer]` The system account that will fund the default account
    ///   2.   `[]` User wallet address (system account, Seed 0)
    ///   3.   `[]` Rent sysvar
    ///   4.   `[]` Seed 1
    ///   5.   `[]` Seed 2
    ///   ...
    Create { size: u64 },

For SPL Token, you'd then give it

    Seed 1 = SPL Token program id
    Seed 2 = Mint address

and you need to add a spl_token::instruction::initialize_account() instruction yourself into the transaction after calling Create.

Assert generalizes to:

    ///   0.   `[writeable]` Default account address derived from `get_default_account_address()`
    ///   1.   `[]` User wallet address (system account, seed 0)
    ///   2.   `[]` Seed 1
    ///   3.   `[]` Seed 2
    ///   ...
    Assert,

[jstarry]

hmm, maybe! How about the DefaultDerivedAccount program, or just DefaultAccount?

Interesting.. by dropping the initialization piece, this Create becomes roughly equivalent to the system create instruction. It's nice that it calculates rent for you too! So, this program then allows us to have an account derivation scheme which when coupled with unsigned initialization is pretty powerful. Using account addresses as seeds is a nice idea too

Create { size: u64 },

Looks good, you want owner as well

Also, maybe DerivedAccount for naming?

[mvines]

hmm, so if Create is generalized then I think Assert goes away entirely.

The main purpose of Assert was to ensure that the user actually owns their default token account. This check can't be supported in general and removing it completely unwinds Assert.

Assert exists because I was worried that a user could be tricked into transferring ownership of their default token account to another party. At which point any token transfers intended for that user's account end up elsewhere. Maybe this is too strict? A wallet should never allow the user to do this so the risk is probably very small in practice. Normal token accounts don't feature this protection.

But without Assert, this program definitely just turns into a variant of SystemInstruction::CreateAccount

[jstarry]

But without Assert, this program definitely just turns into a variant of SystemInstruction::CreateAccount

Yeah, it just reinvents SystemInstruction::CreateAccountWithSeed so I don't it's useful. Thanks for exploring the generalized case, I think adding safe guards is the way to go if we want to establish a convention across all wallets.

At which point any token transfers intended for that user's account end up elsewhere. Maybe this is too strict?

I now feel like it's not strict enough. It feels bad that you could totally brick your default account. Kinda defeats the purpose of having an ecosystem-wide convention. I think the authority has to be controlled by the default token program. What if this becomes a DepositAccountProgram which creates deposit token accounts for users and limits withdraws to the base user account.

[mvines]

@jstarry - wdyt about this morphing into the spl-associated program. I dropped "default", cause it's confusing for users, and tried "derived" -> "associated" since derived is an implementation detail whereas associated I feel makes more sense from a user perspective -- "the token that's associated with my wallet".

The "Assert" instruction is gone and there's only "Create":

pub enum AssociatedInstruction {
    ///   0. `[writeable]` Associated address from `get_default_associated_address()`
    ///   1. `[]` Primary address of the associated account (typically a system account)
    ///   2. `[]` Address of program that will own the associated account
    ///   3. `[writeable,signer]` The system account that will fund the associated account
    ///   4. `[]` System program
    ///   5. `[]` Seed 1
    ///   6. `[]` Seed 2
    ///   7. `[]` Seed 3
    ///   8.      .
    ///   9.      .
    ///  10.      .
    Create {
        /// Number of lamports to transfer to the new account
        lamports: u64,

        /// Number of bytes of memory to allocate
        space: u64,
    }
}

The "default-token-account" then has the SPL Token Account for #2, and the mint this account will
be associated with for #5. Regarding "bricking the account": if you assign the account owner to somewhere else, perhaps you actually wanted to do this, and if not well it's no worse than any other token account.

[jstarry]

Regarding "bricking the account":,
if you assign the account owner to somewhere else, perhaps you actually wanted to do this

but the cost of creating / transferring to a new token account is minimal. I can't imagine an actual need for that

and if not well it's no worse than any other token account.

it is worse because every wallet app is going to be setup to send tokens to that associated account. e.g. All of a sudden when everyone in the family tries to send USDC to grandma they get a strange error because she accidentally gave up ownership of hers and the assert is failing

[mvines]

See #779