[oracle] Add combined vote and prevote tx to reduce block usage#56
[oracle] Add combined vote and prevote tx to reduce block usage#56
Conversation
x/oracle/keeper/msg_server.go
Outdated
| aggregatePrevote, err := ms.GetAggregateExchangeRatePrevote(ctx, valAddr) | ||
| // if there isn't a prevote, we want to no-op the vote so we don't get an error | ||
| // this way, it is safe to use combined vote regardless of a missed vote window | ||
| if err == nil && (uint64(ctx.BlockHeight())/params.VotePeriod)-(aggregatePrevote.SubmitBlock/params.VotePeriod) == 1 { |
There was a problem hiding this comment.
nit we can put (uint64(ctx.BlockHeight())/params.VotePeriod)-(aggregatePrevote.SubmitBlock/params.VotePeriod) == 1 to a helper function called something like IsPrevoteCurrent for readability
|
after removing prevote, how do we guarantee validators won't simply copy others' price feeds by looking at their combinedVote here |
philipsu522
left a comment
philipsu522
left a comment
There was a problem hiding this comment.
let's also add tests.
We'd also want to check the behavior regarding providing a hash and leaving it out
| Args: cobra.RangeArgs(4, 5), | ||
| Short: "Submit an oracle aggregate vote AND prevote for the exchange rates", | ||
| Long: strings.TrimSpace(` | ||
| Submit an oracle aggregate vote and prevote for the exchange rates of the base denom denominated in multiple denoms. The vote is a companian to a prevote from the previous vote window. |
There was a problem hiding this comment.
nit - "while the prevote is for the current window"
There was a problem hiding this comment.
the prevote is performed in the current window, but is for the vote in the next window
|
@LCyson we're not removing prevote, the combined vote performs a vote for the current window, and a prevote for the next window. the prevote for the next window protects against other validators copying the exchange rates. |
what's the workflow for combinedVote in this case? |
|
now it's, val sends combined vote, if there isn't a prevote from previous block, the vote is a no-op, and then prevote for the next block is performed. |
oh ic, so the combinedVote consists of (n - 1) prevote + n vote, that makes sense |
## Describe your changes and provide context * Adds the AnteHandlers Dependency Decorators for the decorators that have read/write operations to/from keepers * Moves the MsgCompletion signaling to the end of RunTx. The main issue is that the signaling SHOULD happen after the write() calls so that dependent transactions have the updated data. * We can make it more granular in the future but for now we should just move it out so that its easier to reason/ ## Testing performed to validate your change With this branch - able to run a couple of rounds of load testing before running into the app hash error due to different gas consumed for the same TX on validators, this is with concurrency enabled #309
## Describe your changes and provide context * Adds the AnteHandlers Dependency Decorators for the decorators that have read/write operations to/from keepers * Moves the MsgCompletion signaling to the end of RunTx. The main issue is that the signaling SHOULD happen after the write() calls so that dependent transactions have the updated data. * We can make it more granular in the future but for now we should just move it out so that its easier to reason/ ## Testing performed to validate your change With this branch - able to run a couple of rounds of load testing before running into the app hash error due to different gas consumed for the same TX on validators, this is with concurrency enabled #309
[SeiDB] Fix MemIAVL Race Condition during snapshot reload
4 participants
I tested this via CLI with localsei to verify behavior. Not sure what the best way to verify REST endpoint behavior is.