ACP-236: Adding auto-renewed validator support#5092
ACP-236: Adding auto-renewed validator support#5092rrazvan1 wants to merge 17 commits intoset-staking-infofrom
Conversation
6e579ef to
b57dc5b
Compare
4330850 to
f7b4f54
Compare
b0be145 to
f5ffba2
Compare
6bef2c6 to
ab9d91c
Compare
af283eb to
ccf880b
Compare
cead098 to
5969322
Compare
| // MulDiv computes (a * b) / c with full precision using big.Int arithmetic. | ||
| // The result is rounded to the nearest integer. | ||
| // Returns ErrDivideByZero if c is zero, or ErrOverflow if the result exceeds uint64. | ||
| func MulDiv(a, b, c uint64) (uint64, error) { | ||
| if c == 0 { | ||
| return 0, ErrDivideByZero | ||
| } | ||
|
|
||
| bigA := new(big.Int).SetUint64(a) | ||
| bigB := new(big.Int).SetUint64(b) | ||
| bigC := new(big.Int).SetUint64(c) | ||
|
|
||
| result := new(big.Int).Mul(bigA, bigB) | ||
| result = divRound(result, bigC) | ||
|
|
||
| if !result.IsUint64() { | ||
| return 0, ErrOverflow | ||
| } | ||
| return result.Uint64(), nil | ||
| } | ||
|
|
||
| // divRound divides a by b and rounds to the nearest integer. | ||
| // Note: This function uses big.Int.DivMod, which has sign-dependent behavior. | ||
| func divRound(a, b *big.Int) *big.Int { | ||
| quotient := new(big.Int) | ||
| remainder := new(big.Int) | ||
|
|
||
| quotient.DivMod(a, b, remainder) | ||
|
|
||
| // if 2*remainder >= b → round up | ||
| doubleRem := new(big.Int).Mul(remainder, big.NewInt(2)) | ||
| if doubleRem.Cmp(b) >= 0 { | ||
| quotient.Add(quotient, big.NewInt(1)) | ||
| } | ||
|
|
||
| return quotient | ||
| } |
There was a problem hiding this comment.
I need to circle back on whether or not we actually need these functions. But if we do keep them, I think the implementation can be cleaned up.
There was a problem hiding this comment.
if we don't care about being specific about the behaviour from the avalanchego implementation itself, we can just use Mul and Div and just add comment about big.Int internal implementation behaviour (I am talking about rounding)
This function is a helper for calculating 2 things:restakingValidationRewards and restakingDelegateeRewards.
But for correctness, we are using it only for computing one side, and the other one is computed through subtraction.
177d85a to
2daf037
Compare
2818a4d to
876c5f0
Compare
| "github.com/ava-labs/avalanchego/vms/secp256k1fx" | ||
|
|
||
| . "github.com/ava-labs/avalanchego/vms/platformvm/validators" | ||
| ) |
There was a problem hiding this comment.
I am surprised not to see any vms/platformvm/ package level tests.
I let CC crunch some computation and it spotted a bug in the base branch set-staking-info that we're basing on.
If we had a package level test, we would've easily caught the bug in the base branch:
// TestAutoRenewedValidatorStakingInfoAfterRenewal verifies that when an
// auto-renewed validator completes a cycle and is renewed (commit path),
// the StakingInfo (AccruedRewards, Period, StakerEndTime, etc.) is preserved
// in the base state after the diff is applied.
//
// This is a regression test for a bug where diff.Apply deleted the
// modifiedStakingInfo entry when removing a validator, even if the same
// validator was also being re-added (renewed) in the same diff.
func TestAutoRenewedValidatorStakingInfoAfterRenewal(t *testing.T) {
require := require.New(t)
vm, _, _ := defaultVM(t, upgradetest.Latest)
vm.ctx.Lock.Lock()
defer vm.ctx.Lock.Unlock()
var (
nodeID = ids.GenerateTestNodeID()
wallet = newWallet(t, vm, walletConfig{})
)
sk, err := localsigner.New()
require.NoError(err)
pop, err := signer.NewProofOfPossession(sk)
require.NoError(err)
// Issue an AddAutoRenewedValidatorTx.
addTx, err := wallet.IssueAddAutoRenewedValidatorTx(
nodeID,
vm.MinValidatorStake,
pop,
vm.ctx.AVAXAssetID,
&secp256k1fx.OutputOwners{
Threshold: 1,
Addrs: []ids.ShortID{ids.GenerateTestShortID()},
},
&secp256k1fx.OutputOwners{
Threshold: 1,
Addrs: []ids.ShortID{ids.GenerateTestShortID()},
},
&secp256k1fx.OutputOwners{
Threshold: 1,
Addrs: []ids.ShortID{ids.GenerateTestShortID()},
},
reward.PercentDenominator/10, // 10% delegation fee
reward.PercentDenominator, // 100% auto-compound
defaultMinStakingDuration,
)
require.NoError(err)
vm.ctx.Lock.Unlock()
require.NoError(vm.issueTxFromRPC(addTx))
vm.ctx.Lock.Lock()
// Accept the AddAutoRenewedValidatorTx in a standard block.
require.NoError(buildAndAcceptStandardBlock(vm))
// Verify the validator was added.
validator, err := vm.state.GetCurrentValidator(constants.PrimaryNetworkID, nodeID)
require.NoError(err)
require.Equal(vm.MinValidatorStake, validator.Weight)
// Verify staking info was set.
stakingInfo, err := vm.state.GetStakingInfo(constants.PrimaryNetworkID, nodeID)
require.NoError(err)
require.Equal(defaultMinStakingDuration, stakingInfo.Period)
require.Equal(uint32(reward.PercentDenominator), stakingInfo.AutoCompoundRewardShares)
// Advance time to the validator's EndTime to trigger renewal.
vm.clock.Set(validator.EndTime)
// Build and accept the proposal block (RewardAutoRenewedValidatorTx)
// choosing the commit (preferred) option, which renews the validator.
require.NoError(buildAndAcceptPreferredOracleBlock(vm))
// After the commit block is accepted and the diff is applied to base
// state, verify the renewed validator exists with updated staking info.
renewedValidator, err := vm.state.GetCurrentValidator(constants.PrimaryNetworkID, nodeID)
require.NoError(err)
// The validator should have been renewed — its StartTime should have advanced.
require.True(renewedValidator.StartTime.After(validator.StartTime),
"renewed validator StartTime (%s) should be after original StartTime (%s)",
renewedValidator.StartTime, validator.StartTime)
// The critical check: StakingInfo must be preserved after renewal.
renewedStakingInfo, err := vm.state.GetStakingInfo(constants.PrimaryNetworkID, nodeID)
require.NoError(err)
// Period must still be set (not zeroed out).
require.Equal(defaultMinStakingDuration, renewedStakingInfo.Period,
"Period should be preserved after renewal, got zero which would cause a graceful exit on next cycle")
// AutoCompoundRewardShares must still be set.
require.Equal(uint32(reward.PercentDenominator), renewedStakingInfo.AutoCompoundRewardShares,
"AutoCompoundRewardShares should be preserved after renewal")
// StakerEndTime must not be epoch (zero).
require.NotZero(renewedStakingInfo.StakerEndTime,
"StakerEndTime should not be zero after renewal")
// If there was a potential reward, AccruedRewards should have increased
// (since AutoCompoundRewardShares is 100%).
if validator.PotentialReward > 0 {
require.NotZero(renewedStakingInfo.AccruedRewards,
"AccruedRewards should be non-zero after renewal with 100%% auto-compound and non-zero PotentialReward")
}
}
I think we're lacking package level tests.
There was a problem hiding this comment.
this has been fixed in set-staking-info branch today, but I need to rebase
There was a problem hiding this comment.
ok but still, I think we should have package level tests for the entire platformVM that go through all steps (wallet → issueTxFromRPC → buildBlock → verify → accept → state) instead of just operating on state diffs directly.
For example, I don't see where we advance the time and build new proposal blocks.
I have asked CC to generate scenarios for tests, and you can ask it to generate tests based on the scenarios (see below).
I think it's worth to consider this.
# Auto-Renewed Validator — Package-Level Test Scenarios
These are VM-level (package `platformvm`) test scenarios for auto-renewed validators (ACP-236).
Each test should go through the full block building and acceptance flow using `defaultVM`,
`newWallet`, `buildAndAcceptStandardBlock`, and `buildAndAcceptPreferredOracleBlock` helpers
in `vm_regression_test.go`.
All tests should use `upgradetest.Latest` (which enables Helicon).
---
## 1. AddAutoRenewedValidatorTx
### 1.1 Happy path: add auto-renewed validator
- Issue `IssueAddAutoRenewedValidatorTx` with valid params (min stake, min duration, 100% auto-compound, 10% delegation fee).
- Build and accept a standard block.
- Verify: validator exists in `vm.state.GetCurrentValidator` with correct weight.
- Verify: `StakingInfo` has correct `Period`, `AutoCompoundRewardShares`, `StakerEndTime`, and zero `AccruedRewards`/`AccruedDelegateeRewards`.
- Verify: `GetCurrentSupply` increased by the validator's `PotentialReward`.
- Verify: the creation tx is retrievable via `vm.state.GetTx`.
### 1.2 Reject duplicate NodeID
- Add an auto-renewed validator for a NodeID.
- Attempt to add another auto-renewed validator for the same NodeID.
- Verify: the second tx is rejected (cannot be issued or block fails verification).
### 1.3 Reject before Helicon
- Use `upgradetest.Fortuna` (Helicon not activated).
- Attempt to issue `AddAutoRenewedValidatorTx`.
- Verify: rejected with Helicon upgrade not active error.
---
## 2. SetAutoRenewedValidatorConfigTx
### 2.1 Change AutoCompoundRewardShares
- Add an auto-renewed validator with `AutoCompoundRewardShares = 1_000_000`.
- Issue `SetAutoRenewedValidatorConfigTx` changing shares to `500_000`.
- Build and accept standard block.
- Verify: `StakingInfo.AutoCompoundRewardShares` is now `500_000`.
- Verify: `Period` unchanged.
### 2.2 Change Period
- Add an auto-renewed validator with `Period = MinStakeDuration`.
- Issue `SetAutoRenewedValidatorConfigTx` changing period to `2 * MinStakeDuration`.
- Build and accept standard block.
- Verify: `StakingInfo.Period` is now `2 * MinStakeDuration`.
### 2.3 Set Period=0 (request graceful exit)
- Add an auto-renewed validator.
- Issue `SetAutoRenewedValidatorConfigTx` with `Period = 0`.
- Build and accept.
- Verify: `StakingInfo.Period` is `0`.
- Advance time to EndTime, build and accept the reward proposal block (commit).
- Verify: validator is **removed** from the current set (graceful exit, not renewed).
- Verify: stake and rewards are returned as UTXOs.
### 2.4 Reject wrong TxID
- Add an auto-renewed validator.
- Issue `SetAutoRenewedValidatorConfigTx` with an incorrect (random) `TxID`.
- Verify: rejected.
### 2.5 Reject wrong owner auth
- Add an auto-renewed validator with a specific `configOwner`.
- Issue `SetAutoRenewedValidatorConfigTx` signed by a different key (not the config owner).
- Verify: rejected.
---
## 3. Restake (Period > 0, Commit Path)
### 3.1 Full auto-compound (100%)
- Add validator with `AutoCompoundRewardShares = 1_000_000` (100%).
- Advance time to EndTime, accept commit block.
- Verify: validator renewed with `Weight = oldWeight + PotentialReward`.
- Verify: `AccruedRewards = PotentialReward` (first cycle).
- Verify: no withdraw reward UTXOs created (everything restaked).
- Verify: `DelegateeReward` reset to 0.
- Verify: `StakerEndTime` advanced by `Period`.
- Verify: `currentSupply` increased by the new cycle's `PotentialReward`.
### 3.2 Zero auto-compound (0%)
- Add validator with `AutoCompoundRewardShares = 0`.
- Advance time to EndTime, accept commit block.
- Verify: validator renewed with **same weight** as before.
- Verify: `AccruedRewards` unchanged (nothing restaked).
- Verify: withdraw UTXO created for full `PotentialReward` to `ValidationRewardsOwner`.
- Verify: new cycle starts with same weight.
### 3.3 Partial auto-compound (e.g., 30%)
- Add validator with `AutoCompoundRewardShares = 300_000` (30%).
- Advance time, accept commit.
- Verify: 30% of `PotentialReward` restaked (added to weight and `AccruedRewards`).
- Verify: 70% withdrawn as UTXO to `ValidationRewardsOwner`.
### 3.4 Restake with delegatee rewards
- Add auto-renewed validator.
- Add a delegator that ends before the validator's cycle end.
- Advance time to delegator EndTime, accept commit (rewards delegator, accumulates `DelegateeReward`).
- Advance time to validator EndTime, accept commit.
- Verify: `DelegateeReward` was split by `AutoCompoundRewardShares`.
- Verify: restaked portion added to `AccruedDelegateeRewards` and weight.
- Verify: withdrawn portion as UTXO to `DelegationRewardsOwner`.
- Verify: `DelegateeReward` reset to 0 after renewal.
### 3.5 Multiple cycles — reward compounding
- Add validator with 100% auto-compound.
- Run 3 cycles (advance time, accept commit each time).
- Verify after each cycle:
- Weight increases (previous rewards compound).
- `AccruedRewards` accumulates across all cycles.
- New `PotentialReward` is calculated with the updated weight.
- `currentSupply` tracks correctly.
### 3.6 Config change mid-cycle takes effect at next cycle end
- Add validator with `AutoCompoundRewardShares = 1_000_000`.
- Mid-cycle, issue `SetAutoRenewedValidatorConfigTx` changing shares to `0`.
- Advance time to cycle end, accept commit.
- Verify: 0% restaked (new shares applied), all rewards withdrawn.
---
## 4. Overflow (Weight Exceeds MaxValidatorStake)
### 4.1 Restaking exceeds MaxValidatorStake
- Add validator with `Weight = MaxValidatorStake - smallAmount`.
- Ensure `PotentialReward > smallAmount` so restaking would exceed max.
- Advance time, accept commit.
- Verify: `Weight == MaxValidatorStake` (capped).
- Verify: excess rewards withdrawn as UTXOs.
- Verify: `AccruedRewards` reflects only the portion that was restaked.
### 4.2 Already at MaxValidatorStake
- Add validator at exactly `MaxValidatorStake`.
- Advance time, accept commit.
- Verify: all restaking rewards overflow → withdrawn as UTXOs.
- Verify: weight remains at `MaxValidatorStake`.
---
## 5. Abort Path (Period > 0)
### 5.1 Abort first cycle
- Add validator, immediately abort at first cycle end.
- Verify: validator **removed** from current set.
- Verify: only stake returned (no accrued reward UTXOs).
- Verify: `PotentialReward` forfeited.
- Verify: `currentSupply` decremented by forfeited `PotentialReward`.
### 5.2 Abort returns stake and accrued rewards
- Add validator with 100% auto-compound.
- Run one commit cycle (accumulates `AccruedRewards`).
- On the second cycle, force abort (use non-preferred option).
- Verify: validator **removed** from current set.
- Verify: stake UTXOs returned.
- Verify: `AccruedRewards` from prior cycle returned as UTXO to `ValidationRewardsOwner`.
- Verify: current cycle's `PotentialReward` **forfeited** (no UTXO).
- Verify: `currentSupply` decremented by forfeited `PotentialReward`.
---
## 6. Graceful Exit (Period == 0)
### 6.1 Graceful exit on commit — first cycle
- Add validator, set `Period = 0` via `SetAutoRenewedValidatorConfigTx`.
- Advance time to EndTime, accept commit.
- Verify: validator removed.
- Verify: stake returned.
- Verify: `PotentialReward` paid as UTXO (since commit = eligible).
- Verify: no accrued rewards (first cycle).
### 6.2 Graceful exit on commit — after multiple cycles
- Add validator with 100% auto-compound.
- Run 2 commit cycles (accumulates `AccruedRewards` and `AccruedDelegateeRewards`).
- Set `Period = 0`.
- Advance time, accept commit.
- Verify: stake returned.
- Verify: `PotentialReward + AccruedRewards` paid as single UTXO to `ValidationRewardsOwner`.
- Verify: `DelegateeReward + AccruedDelegateeRewards` paid as single UTXO to `DelegationRewardsOwner`.
- Verify: validator removed.
### 6.3 Graceful exit on abort
- Same setup as 6.2 but accept abort instead of commit.
- Verify: stake returned.
- Verify: only `AccruedRewards` paid (no `PotentialReward`).
- Verify: `DelegateeReward + AccruedDelegateeRewards` still paid (delegatee rewards always paid).
- Verify: `currentSupply` decremented by forfeited `PotentialReward`.
---
## 7. State Persistence and Diff Apply
### 7.1 StakingInfo preserved after renewal (regression test)
- Add validator with 100% auto-compound.
- Advance time, accept commit (renewal).
- Verify in `vm.state` (base state after Apply):
- `Period` is preserved (not zeroed).
- `AccruedRewards` is non-zero.
- `AutoCompoundRewardShares` is preserved.
- `StakerEndTime` is non-zero and correct.
### 7.2 StakingInfo cleared on pure removal
- Add validator, set Period=0 (graceful exit).
- Advance time, accept commit.
- Verify: validator no longer in current set.
- Verify: `GetStakingInfo` returns `ErrNotFound`.
### 7.3 Weight reconstruction after restart
- Add validator with 100% auto-compound.
- Run 2 commit cycles (weight should grow).
- Simulate state reload: call `vm.state.Commit()`, then verify `GetCurrentValidator` returns correct compounded weight.
---
## 8. Block Builder and Reward Tx Generation
### 8.1 Block builder creates RewardAutoRenewedValidatorTx
- Add auto-renewed validator.
- Advance time to EndTime.
- Call `vm.Builder.BuildBlock`.
- Verify: the built block builds and accepts successfully.
- Verify: the reward tx has correct `TxID` and `Timestamp`.
### 8.2 RewardValidatorTx rejected for auto-renewed validator
- Add auto-renewed validator.
- Advance time.
- Verify the block builder creates the correct tx type and the validator is renewed.
---
## 9. Supply Tracking
### 9.1 Supply increases on add
- Before and after `AddAutoRenewedValidatorTx`, verify `currentSupply` increased by `PotentialReward`.
### 9.2 Supply adjusted on commit restake
- Old `PotentialReward` stays in supply (was already added).
- New `PotentialReward` added for the next cycle.
- Verify: `currentSupply` after commit = before + newPotentialReward.
### 9.3 Supply decremented on abort
- Verify: `currentSupply` after abort = before - PotentialReward.
### 9.4 Supply correct on graceful exit commit
- Verify: `currentSupply` unchanged (PotentialReward stays — it was already in supply and is now paid out).
### 9.5 Supply correct on graceful exit abort
- Verify: `currentSupply` decremented by `PotentialReward`.
---
## 10. Delegator Interaction
### 10.1 Delegator rewards accumulate and reset at cycle end
- Add auto-renewed validator with delegation fee.
- Add a delegator that ends at the same time as the validator's cycle.
- Accept delegator reward (commit), then accept validator reward (commit).
- Verify: `DelegateeReward` resets to 0 after validator renewal.
- Verify: any delegatee fee is in `AccruedDelegateeRewards`.
---
## 11. UTXO Correctness
### 11.1 Reward UTXOs indexed correctly
- After any reward (commit or abort), verify:
- `GetRewardUTXOs(validator.TxID)` returns the expected UTXOs.
- Each UTXO has the correct asset, amount, and owner.
### 11.2 Stake UTXOs match original StakeOuts
- On abort or graceful exit, verify:
- Stake UTXOs match the original `StakeOuts` amounts and owners.
- Output indices are based on the original creation tx ID.
|
|
||
| if e.backend.Config.PartialSyncPrimaryNetwork && | ||
| tx.NodeID() == e.backend.Ctx.NodeID { | ||
| e.backend.Ctx.Log.Warn("verified transaction that would cause this node to become unhealthy", |
There was a problem hiding this comment.
I don't understand what's so special about logging this under e.backend.Config.PartialSyncPrimaryNetwork.
This flag just means we exclude syncing the C and X chains.
Shouldn't this be logged anyway regardless of this flag being set?
There was a problem hiding this comment.
afaiu, a node with this flag enabled cannot participate in consensus, so a transaction adding the local node as a validator is triggering a warning.
There is also a healthcheck that marks the node as unhealthy if it has this flag enabled + is a validator.
Line 1478 in 77c2d36
There was a problem hiding this comment.
afaiu, a node with this flag enabled cannot participate in consensus, so a transaction adding the local node as a validator is triggering a warning.
This flag means it syncs the P-chain only, not C and X chains.
I think we still participate in consensus (of the P-chain) because if we're a validator, then other nodes sample us regardless of if we have this flag or not.
I think I understand now - essentially this is a warning because we're being added as a validator but we're only replicating the P-chain.
This also exists in the regular non-renewed validator flow.
I think it's weird to have this warning when we have the health check, but let's leave it as is
|
Posted some suggestions here #5140 |
…ean up service.go, and add restake test
a68f30d to
10ff006
Compare
| if err != nil { | ||
| return nil, err | ||
| } | ||
| delegatorOwnerComplexity, err := fee.OwnerComplexity(delegationRewardsOwner) |
There was a problem hiding this comment.
is it not possible to re-use AddAutoRenewedValidatorTx of complexity.go to execute the code in this function?
There was a problem hiding this comment.
complexityVisitor isn't exported, so I think it requires things out of scope of this PR
| return nil, err | ||
| } | ||
|
|
||
| utils.Sort(validationRewardsOwner.Addrs) |
There was a problem hiding this comment.
what is the reason for sorting?
There was a problem hiding this comment.
canonical representation of the tx
63a7e29 to
c5407e4
Compare
|
|
||
| bigA := new(big.Int).SetUint64(a) | ||
| bigB := new(big.Int).SetUint64(b) | ||
| bigC := new(big.Int).SetUint64(c) | ||
|
|
||
| result := new(big.Int).Mul(bigA, bigB) | ||
| result = divRound(result, bigC) | ||
|
|
||
| if !result.IsUint64() { | ||
| return 0, ErrOverflow | ||
| } | ||
| return result.Uint64(), nil |
There was a problem hiding this comment.
| bigA := new(big.Int).SetUint64(a) | |
| bigB := new(big.Int).SetUint64(b) | |
| bigC := new(big.Int).SetUint64(c) | |
| result := new(big.Int).Mul(bigA, bigB) | |
| result = divRound(result, bigC) | |
| if !result.IsUint64() { | |
| return 0, ErrOverflow | |
| } | |
| return result.Uint64(), nil | |
| hi, lo := bits.Mul64(a, b) | |
| if c <= hi { | |
| return 0, ErrOverflow | |
| } | |
| quo, rem := bits.Div64(hi, lo, c) | |
| if rem < (1<<63) && 2*rem < c { | |
| return quo, nil | |
| } | |
| if quo == math.MaxUint64 { | |
| return 0, ErrOverflow | |
| } | |
| return quo + 1, nil |
This doesn't require using big.Int as the bits package provides full-precision alternatives. The divRound() function becomes redundant too.
There was a problem hiding this comment.
nice suggestion! For now, I will update with your approach and we can still consider the way we calculate it as an open discussion if there are better ideas out there.
Why this should be merged
Implements ACP-236 (Auto-Renewed Staking) for the primary network. This enables validators to remain staked indefinitely without manually renewing at the end of each period.
How this works
Mutable validator state (accrued rewards, config) is stored in
StakingInfometadata structure, separate from the immutable Staker record.Introduces three new transaction types gated behind the Helicon upgrade:
See
vms/platformvm/docs/validators_auto_renewed.mdfor the full implementation spec.How this was tested
Unit tests and E2E tests (#4971)
Need to be documented in RELEASES.md?
Yes.