Repository navigation
feat(vm): optimize voting power management (TIP-944) - #7068
yanghang8612 wants to merge 5 commits into
Conversation
| assertEquals(1, dbManager.getDynamicPropertiesStore().getAllowTvmVotePowerOptimization()); | ||
| assertThrows(ContractValidateException.class, () -> ProposalUtil.validator( | ||
| dbManager.getDynamicPropertiesStore(), fork, 103, 1)); | ||
| ConfigLoader.disable = false; |
There was a problem hiding this comment.
The governance test is a nice end-to-end check: fork gating, one-way activation, ProposalService application and both ConfigLoader modes are all exercised in one place.
One thing worth tightening: the test sets ConfigLoader.disable = false and finishes while the global VMConfig snapshot still reports allowTvmVotePowerOptimization == true. Both are static, so this state can leak into other test classes that share the same JVM fork and make unrelated assertions pass or fail for the wrong reason. Would it be worth restoring ConfigLoader.disable and the global snapshot in an @After so the class is self-contained?
There was a problem hiding this comment.
Thanks for checking the test isolation. Tip944Test inherits the method-level VMConfigRule from BaseTest. That rule already deep-copies and restores the global snapshot and ConfigLoader.disable, and clears the thread-local view, including when setup or the test fails.
I've added three focused regressions covering normal completion, setup failure, and assertion failure, with in-place flag mutation and snapshot replacement. They verify all snapshot fields, loader state, and local-view cleanup. The focused suites pass on both TIP branches, including the governance tests.
There was a problem hiding this comment.
Thanks for the clarification and the additional tests. I checked VMConfigRule and confirmed that it restores the configuration after each test, including when setup or an assertion fails. This addresses my concern.
|
Could this leave a historical account with stale A possible pre-activation sequence, assuming the relevant feature flags are enabled:
This PR appears to prevent that state from being created by the same sequence after activation. However, for an account already in that state, both new self-destruct invalidation checks appear to be skipped because they require positive stake. The optimized vote-cleanup path also no longer resets Could such an account retain the stale snapshot and subsequently vote through I have not reproduced this end to end or confirmed that such accounts exist on-chain. Would it be worth adding a regression test starting with zero stake and positive |
What does this PR do?
Implement TIP-944 behind
ALLOW_TVM_VOTE_POWER_OPTIMIZATION(103), aligning legacy voting-power state transitions for TVM voting, Stake 1.0 freezing/unfreezing, and self-destruct stake receipt and cleanup.Why are these changes required?
Keep TVM account-state transitions consistent with native NewResourceModel rules.
Related to TIP-944.
This PR has been tested by:
./gradlew clean build -x teston macOS ARM64 / JDK 17..github/scripts/check_math_usage.sh.Follow up
Coordinate on-chain activation. Existing feature prerequisites and voting-power selection conditions remain applicable; no historical account-state migration is introduced.
Extra details
Targets
release_v4.8.3. Proposal validation, application, and parameter output place 103 after 102. TIP-945 has independent runtime activation.