Skip to content

fix: create vote should use create_account_with_seed if applicable#2819

Closed
Managarmrr wants to merge 1 commit intoanza-xyz:masterfrom
Managarmrr:patches/vote_account_with_seed
Closed

fix: create vote should use create_account_with_seed if applicable#2819
Managarmrr wants to merge 1 commit intoanza-xyz:masterfrom
Managarmrr:patches/vote_account_with_seed

Conversation

@Managarmrr
Copy link
Copy Markdown

Problem

When creating a vote account using the cli system_instruction::create_account has been used in any case regardless of whether a seed has been provided or not.

This works if the base the vote account is derived from is the included in the transaction for any other reason (such as being the fee payer), but not if it is solely included as the base account.

By using `system_instruction::create_account_with_seed this issue has been mitigated.

Summary of Changes

Use system_instruction::create_account_with_seed instead of system_instruction::create_account if derived from a seed.

When creating a vote account using the cli system_instruction::create_account
has been used in any case regardless of whether a seed has been provided or not.

This works if the base the vote account is derived from is the included in the
transaction for any other reason (such as being the fee payer), but not if it
is solely included as the base account.

By using system_instruction::create_account_with_seed this issue has been mitigated.
@mergify
Copy link
Copy Markdown

mergify Bot commented Sep 3, 2024

The Firedancer team maintains a line-for-line reimplementation of the
native programs, and until native programs are moved to BPF, those
implementations must exactly match their Agave counterparts.
If this PR represents a change to a native program implementation (not
tests), please include a reviewer from the Firedancer team. And please
keep refactors to a minimum.

@mergify mergify Bot requested a review from a team September 3, 2024 11:13
Copy link
Copy Markdown

@buffalojoec buffalojoec left a comment

Choose a reason for hiding this comment

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

Nice find. It looks like CreateVoteAccountConfig is used in a bunch of tests all over the repo, but they only use the space field, never with_seed.

Any chance you'd mind adding a test to programs/vote/src/vote_processor.rs for actually using the with_seed arg?

@buffalojoec
Copy link
Copy Markdown

The Firedancer team maintains a line-for-line reimplementation of the native programs, and until native programs are moved to BPF, those implementations must exactly match their Agave counterparts. If this PR represents a change to a native program implementation (not tests), please include a reviewer from the Firedancer team. And please keep refactors to a minimum.

This is only a change to the instruction-builder helper, not the program itself. As per my review comment, tests may also be added. Program processor, state, and instructions are unchanged.

@joncinque
Copy link
Copy Markdown

This PR contains changes to the solana sdk, which will be moved to a new repo within the next week, when v2.2 is branched from master.

Please merge or close this PR as soon as possible, or re-create the sdk changes when the new repository is ready at https://github.com/anza-xyz/solana-sdk

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants