improve: add support of compactUpdateVoteState instruction #2704
Conversation
|
| voteAccount: instruction.keys[0].pubkey, | ||
| voteAuthority: instruction.keys[1].pubkey, | ||
| voteStateUpdate: { | ||
| lockouts: lockoutOffsets.map( |
There was a problem hiding this comment.
this transformation is made to be compliant with the output provided by api.mainnet-beta.solana.com, since node returns lockouts, while instruction for "compact" contains offset values.
There was a problem hiding this comment.
I see what you're going for here. For anyone else reading, this is required due to the custom Serde serialization method used for CompactUpdateVoteState, ie. serde_compact_vote_state_update.
Since this is required at a serialization level, and you'll use this twice (here and in the transaction function), why not roll this into a custom serialization layout?
You can define a class MyLayout extends Layout<T> and implement the required methods that can transform from the instruction input type to the proper byte layout, and back.
We did this in SPL Token JS to mimic Rust's COption::<Pubkey> and you can use that as an example.
https://github.com/solana-labs/solana-program-library/blob/d1c03cc6164ebdeebef19190de28b7a71f4e4cb6/token/js/src/serialization.ts#L5-L39
| @@ -96,6 +97,17 @@ export type UpdateValidatorIdentityParams = { | |||
| nodePubkey: PublicKey; | |||
| }; | |||
|
|
|||
| export type CompactUpdateVoteStateParams = { | |||
There was a problem hiding this comment.
nit: can you document this type, like the other parameter types?
| @@ -96,6 +97,17 @@ export type UpdateValidatorIdentityParams = { | |||
| nodePubkey: PublicKey; | |||
| }; | |||
|
|
|||
| export type CompactUpdateVoteStateParams = { | |||
| voteAccount: PublicKey, | |||
There was a problem hiding this comment.
nit: Just to follow the pattern of the others:
| voteAccount: PublicKey, | |
| votePubkey: PublicKey, |
| voteStateUpdate: { | ||
| lockouts: Lockout[], | ||
| root: number, | ||
| hash: PublicKey, |
There was a problem hiding this comment.
Unfortunately, the legacy library doesn't have an opaque type for a hash, but it looks like the rest of the package uses string. It definitely can't be PublicKey here, for a few reasons.
However, when it comes to decoding, you can use the publicKey layout, since it will perform base58 decoding and assert the resulting length is 32 bytes.
| @@ -342,6 +402,23 @@ const VOTE_INSTRUCTION_LAYOUTS = Object.freeze<{ | |||
| Layout.voteAuthorizeWithSeedArgs(), | |||
| ]), | |||
| }, | |||
| CompactUpdateVoteState: { | |||
| index: 12, | |||
| layout: BufferLayout.struct<VoteInstructionInputData['CompactUpdateVoteState']>([ | |||
There was a problem hiding this comment.
You can add this whole layout to layout.ts and then just call Layout.voteCompactUpdateVoteState() here.
|
|
||
| const keys = [ | ||
| {pubkey: voteAccount, isSigner: false, isWritable: true}, | ||
| {pubkey: voteAuthority, isSigner: true, isWritable: true}, |
There was a problem hiding this comment.
Docs on this instruction only declare the authority as signer, not writable.
| voteAccount: instruction.keys[0].pubkey, | ||
| voteAuthority: instruction.keys[1].pubkey, | ||
| voteStateUpdate: { | ||
| lockouts: lockoutOffsets.map( |
There was a problem hiding this comment.
I see what you're going for here. For anyone else reading, this is required due to the custom Serde serialization method used for CompactUpdateVoteState, ie. serde_compact_vote_state_update.
Since this is required at a serialization level, and you'll use this twice (here and in the transaction function), why not roll this into a custom serialization layout?
You can define a class MyLayout extends Layout<T> and implement the required methods that can transform from the instruction input type to the proper byte layout, and back.
We did this in SPL Token JS to mimic Rust's COption::<Pubkey> and you can use that as an example.
https://github.com/solana-labs/solana-program-library/blob/d1c03cc6164ebdeebef19190de28b7a71f4e4cb6/token/js/src/serialization.ts#L5-L39
|
@buffalojoec, thank you for the review, I'll handle the feedback in the closest days. |
No description provided.