Skip to content

Conversation

@dedeleono
Copy link
Contributor

@dedeleono dedeleono commented Nov 9, 2025

This update builds the instruction with a const [4, 0, 0, 0] payload, ensuring the System Program receives the 4-byte little-endian discriminator it expects. This prevents the invalid instruction data errors previously occurring.

Ref: Issue #270

Copy link
Contributor

@sonicfromnewyoke sonicfromnewyoke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good catch, system program uses u32 le as discriminator

also, would you mind updating upgrade_nonce_account.rs and making the same change?

program_id: &crate::ID,
accounts: &account_metas,
data: &[4],
data: &DATA,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe this way will be simpler?

Suggested change
data: &DATA,
data: &[4, 0, 0, 0],

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

true haha, just made a new commit with the change and also updated upgrade_nonce_account.rs


// instruction
// instruction data uses 4-byte LE discriminator expected by the system program
const DATA: [u8; 4] = [4, 0, 0, 0];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use this array directly

Suggested change
const DATA: [u8; 4] = [4, 0, 0, 0];

Copy link
Contributor

@sonicfromnewyoke sonicfromnewyoke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

];

// instruction
// instruction data uses 4-byte LE discriminator expected by the system program
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Could we keep the existing comment? I think it is a bit redundant to have it on every instruction and other don't have it.

let account_metas: [AccountMeta; 1] = [AccountMeta::writable(self.account.key())];

// instruction
// instruction data uses 4-byte LE discriminator expected by the system program
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Same here, could we keep the existing comment?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, just pushed the new commit to keep the existing comment

Copy link
Collaborator

@febo febo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great, thank you! I will fix the CI issue so we can get this merged.

@febo
Copy link
Collaborator

febo commented Nov 17, 2025

@dedeleono If you rebase the PR, CI should be able to run.

@dedeleono dedeleono force-pushed the advance_nonce_account_fix branch from 6991914 to e892ebe Compare November 17, 2025 14:10
@dedeleono
Copy link
Contributor Author

dedeleono commented Nov 17, 2025

Update

Nice, just did! @febo

Copy link
Collaborator

@febo febo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great, thanks!

@febo febo merged commit 05f74a9 into anza-xyz:main Nov 17, 2025
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants