trezor / trezor-suite

Trezor Suite Monorepo
https://trezor.io/trezor-suite
Other
737 stars 255 forks source link

Trezor rejects Solana transaction for signing when setting authority to null to disable minting #12134

Open concernedrat opened 7 months ago

concernedrat commented 7 months ago

Issue Title:
Trezor rejects transaction when setting authority to null to disable minting

Description:
When attempting to disable minting by setting the new authority to null as per the Solana SPL SDK, Trezor rejects the transaction, marking it as invalid (the signature of the createSetAuthorityInstructions accepts PublicKey | null for this field). This behavior is inconsistent with the expected behavior from the SDK and Solana.

Steps to Reproduce:

  1. Create a transaction to set authority for disabling minting.
  2. Set the new authority to null.
  3. Sign the transaction using Trezor.
  4. Trezor marks the transaction as invalid.

Expected Behavior:
Trezor should accept the transaction even when the new authority is set to null, as it is a valid instruction according to the Solana SPL SDK.

Code Snippet:

// Create a new transaction
let transaction = new Transaction();
transaction.feePayer = payer; 
transaction.recentBlockhash = (await connection.getRecentBlockhash()).blockhash;

// Create the instruction to set authority
const instruction = createSetAuthorityInstruction(
    mintPublicKey,
    currentAuthority,
    authorityType,
    null, // New authority (null in this case)
);

transaction.add(instruction);

const signedTransaction = await trezorSigner(transaction, "m/44'/501'/0'/0'", payer);

Additional Information:
This issue prevents users from programmatically disabling minting using Trezor. As a workaround I set up the new authority to a dummy account 11111111111111111111111111111111 and mark the token account as immutable to disable minting (not ideal)

MiroslavProchazka commented 7 months ago

Thanks for reporting it! I've asked @martykan to investigate it further.

martykan commented 7 months ago

I have reproduced the issue, the problem needs to be fixed on the firmware side, it's from the definitions in this file: https://github.com/trezor/trezor-firmware/blob/main/core/src/apps/solana/transaction/instructions.py

@Hannsek

concernedrat commented 7 months ago

@MiroslavProchazka thank you guys for the amazing work! 🫶🏽

I briefly dug into https://github.com/trezor/trezor-firmware/blob/c635b945e175762be93febbee6b983317b900c6e/core/src/apps/solana/transaction/instructions.py#L3082 looking to shed some light and new_authority field of that instruction is correctly set to optional (is_optional: true)

Cheers!

matejcik commented 6 months ago

@concernedrat can you provide binary serialization of the transaction that you end up with?