Send a validator set update only once per epoch #725
Conversation
|
Closes #492 |
| test-unit-debug: | ||
| $(debug-cargo) test \ | ||
| $(TEST_FILTER) -- \ | ||
| --skip e2e \ | ||
| --nocapture \ | ||
| -Z unstable-options --report-time | ||
|
|
There was a problem hiding this comment.
Re. call yday, this should be a separate PR (against main) as well/instead
There was a problem hiding this comment.
yeah I can create a small PR for main
| fn can_send_validator_set_update(&self, _can_send: SendValsetUpd) -> bool { | ||
| // TODO: implement this method for ABCI++; should only be able to send | ||
| // a validator set update at the second block of an epoch | ||
| true | ||
| } | ||
|
|
||
| // handle genesis block corner case | ||
| if height == BlockHeight(1) { | ||
| #[cfg(not(feature = "abcipp"))] | ||
| fn can_send_validator_set_update(&self, can_send: SendValsetUpd) -> bool { | ||
| // when checking vote extensions in Prepare | ||
| // and ProcessProposal, we simply return true | ||
| if matches!(can_send, SendValsetUpd::AtPrevHeight) { | ||
| return true; | ||
| } |
There was a problem hiding this comment.
We can get rid of the SendValsetUpd now, we no longer need this context from the caller? can_send_validator_set_update can reason by itself using only the current decision height.
There was a problem hiding this comment.
we call can_send_validator_set_update from two places:
- when we create a new vote extension at the ABCI commit phase, where we pass
SendValsetUpd::Nowas arg - when we verify vote extensions e.g. at prepare and process proposal, where we pass
SendValsetUpd::AtPrevHeightas arg
we still need to distinguish between these two cases; a BlockHeight parameter wouldn't be enough, since we need SendValsetUpd::AtPrevHeight to always return true. no parameters wouldn't work either, because you need to check the prev height case
There was a problem hiding this comment.
you could remove it altogether if we refactored prepare and process proposal to always accept validator set update protocol txs without a predicate check, but then it'd be kind of annoying to write this code again for ABCI++ if/when that happens...
There was a problem hiding this comment.
you could remove it altogether if we refactored prepare and process proposal to always accept validator set update protocol txs without a predicate check, but then it'd be kind of annoying to write this code again for ABCI++ if/when that happens...
I would actually do this but since we're going to maintain our current abcipp code I guess it's fine
|
plz merge only after #723 |
Instead of spamming the chain with validator set update protocol txs, now we only send these once per epoch, at the second block height offset within the epoch.