codama-idl / codama

Generate clients, CLIs, documentation and more from your Solana programs
MIT License
103 stars 19 forks source link

PDA derivation with string seed incorrect for Anchor #337

Open swimricky opened 4 days ago

swimricky commented 4 days ago

https://github.com/solana-program/create-solana-program/issues/108

if you have an Anchor program like the following

 #[program]
mod drp_test_scaffold {
    use super::*;

    pub fn create(ctx: Context<Create>, name: String, authority: Pubkey) -> Result<()> {
        let counter = &mut ctx.accounts.counter;
        counter.name = name;
        counter.authority = authority;
        counter.count = 0;
        Ok(())
    }
}

#[derive(Accounts)]
#[instruction(name: String)]
pub struct Create<'info> {
    #[account(
        init, 
        seeds = [name.as_bytes()],
        bump,
        payer = payer, space = 8 + Counter::INIT_SPACE
    )]
    pub counter: Account<'info, Counter>,
    #[account(mut)]
    pub payer: Signer<'info>,
    pub system_program: Program<'info, System>,
}

the generated Encoder for the seeds is incorrect. it generates the following snippet for resolving the counter account if not provided

 // Resolve default values.
  if (!accounts.counter.value) {
    accounts.counter.value = await getProgramDerivedAddress({
      programAddress,
      seeds: [
        addEncoderSizePrefix(getUtf8Encoder(), getU32Encoder()).encode(
          expectSome(args.name)
        ),
      ],
    });
  }

it should be using

 getUtf8Encoder().encode(
          expectSome(args.name)
        ),
lorisleiva commented 1 day ago

Thanks for opening a separate issue.

I've added my thoughts on this bug here. Copy/pasting below to close the other thread:


Thanks for the detailed updates guys!

Yes, I can see now that the Anchor macro says seeds = [name.as_bytes()], which basically means "remove the u32 size prefix of the String type.

The really annoying thing is that this is actually not exposed on the IDL whatsoever.

"pda": {
  "seeds": [
    {
      "kind": "arg",
      "path": "name" // argument "name" is `string`, not `string.as_bytes()` or something.
    }
  ]
}

So from the perspective of Codama, you're just getting the string type which, by default with Anchor, is a u32 prefixed string.

Since I believe this is the way most people using string as seeds anyway, it may be okay to just assume that string seed should never have any prefix. But that would introduce a bug for anyone that currently uses u32 prefixed strings for seeds.

I think it's still worth doing but maybe best to release it in 2.0. 🤔