Forbid changing staker's destination in ProduceBlockFromStake after a fork #1898
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
azarovh
force-pushed
the
feat/freeze_order
branch
from
3873c8b to
c092766
Compare
ImplOfAnImpl
force-pushed
the
forbid_staking_destination_change
branch
2 times, most recently
from
4edb286 to
bcd3833
Compare
azarovh
force-pushed
the
feat/freeze_order
branch
2 times, most recently
from
3ab75a5 to
cc0087c
Compare
ImplOfAnImpl
force-pushed
the
forbid_staking_destination_change
branch
from
bcd3833 to
b62ee3a
Compare
azarovh
approved these changes
Apr 9, 2025
ImplOfAnImpl
force-pushed
the
forbid_staking_destination_change
branch
from
363aa29 to
b178e6a
Compare
…mmon-test-helpers` crate; introduce `ChainstateUpgradesBuilder` and use it in the `ChainConfig` builder
OBorce
approved these changes
Apr 18, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Some notes:
The changes in
consensusare cosmetical (I reverted the code that needed them, but decided to keep the cleanup part).I removed the used trait
Activateincommon.I also movedChainstateUpgradeBuilderto a separate cratecommon-test-helpersin "common/test-helpers" to be able to re-use it in other places.Why it's called "helpers":1) We already have thep2p-test-utilscrate in "p2p/test-utils", which does NOT depend onp2p. Andcommon-test-helpersDOES depend oncommon. So I felt like they need to use different naming schemes, so that one kind of crate is distinguishable from the other one and so that it's possible to have both kinds in the same "parent" crate.2) We also have the "test_helpers" module inp2p, which lives inside production code, but is intended to be used by tests. The module does use stuff fromp2p, obviously, so I decided to re-use the word "helpers" for the new crate too.I suggest using this naming approach for the future crates too:1) "xxx-test-helpers" - can use "xxx" (and therefore cannot be used in xxx's unit tests).2) "xxx-test-utils" - cannot use "xxx" (and therefore can be used in xxx's unit tests).And a test-only module inside production code should be named just "test-helpers"Edit: I'm now actually thinking about movingChainstateUpgradeBuilderto production code, so that in can be re-used incommon/src/chain/config/builder.rs. I don't want to do it in this PR (to avoid merge conflicts inmake_token_idshould not generate id from account inputs #1908), but I think I'll do it in a subsequent PR. In that case thecommon-test-helperscrate will become empty and will be removed.The general suggestion about the naming scheme still stands.
But I moved
ChainstateUpgradeBuilderto the production code and removed thecommon-test-helperscrate for now.I also introduced ChainstateUpgradesBuilder that uses ChainstateUpgradeBuilder to build the upgrades incrementally. IMO now it's a bit easier to see what exactly was changes at each particular height.