tpr: extend to lqt actions #5080
Conversation
| // The address the user wants potential rewards to go to. | ||
| core.keys.v1.Address rewards_recipient = 2; | ||
| // The note containing the staked note used for voting. | ||
| SpendableNoteRecord staked_note = 3; |
There was a problem hiding this comment.
probably only need Note type here, but taking an entire SNR object seems fine.
There was a problem hiding this comment.
If we take a SpendableNoteRecord then we shouldn't take the position of the note as well, since the SNR contains the position
There was a problem hiding this comment.
good catch, kept SpendableNoteRecord and removed note position
There was a problem hiding this comment.
should the request actually accept Vec<SpendableNoteRecord>? what if the user's voting weight exceeds a single note and spans multiple notes?
There was a problem hiding this comment.
yeah I think having a repeated here makes sense
There was a problem hiding this comment.
yeah I think having a repeated here makes sense
modified accordingly 13e82f2
| // The position of the staked note. | ||
| uint64 staked_note_position = 4; | ||
| // The start position of the tournament. | ||
| uint64 start_position = 5; |
There was a problem hiding this comment.
I think we should take an epoch index here (same type, different field name) so that we match the plan. Also, any position should always be (epoch, 0, 0) so it might as well be the epoch.
There was a problem hiding this comment.
modified field name to epoch_index
Describe your changes
extends
TransactionPlannerRequestto supportActionLiquidityTournamentVotevariant, which is necessary for wasm testing.Issue ticket number and link
Checklist before requesting a review
I have added guiding text to explain how a reviewer should test these changes.
If this code contains consensus-breaking changes, I have added the "consensus-breaking" label. Otherwise, I declare my belief that there are not consensus-breaking changes, for the following reason: