TIP-1017 Validator Config V2#2181
Conversation
e61f62b to
9824220
Compare
920281b to
983e145
Compare
983e145 to
7ec1892
Compare
Co-authored-by: dankrad <mail@dankradfeist.de>
57d29ca to
cbf2d15
Compare
| at the boundary height: | ||
|
|
||
| ``` | ||
| if chainspec.is_allegro_active_at_timestamp(block.timestamp) { |
There was a problem hiding this comment.
| if chainspec.is_allegro_active_at_timestamp(block.timestamp) { | |
| if chainspec.is_hardfork_active_at_timestamp(contract_upgrade_hardfork, block.timestamp) { |
There was a problem hiding this comment.
Why the extra contract_upgrade_hardfork argument?
## Summary **Manual migration**: Admin migrates validators one at a time via `migrateValidator(valIdx)`, then calls `initialize()` to flip the flag. CL reads from V1 until the flag is set. | Change | Description | |--------|-------------| | **Read before init** | CL reads directly from V1 (no proxying in V2) | | **migrateValidator(valIdx)** | Migrates single validator; copies owner on first call; reverts if `valIdx != validatorsArray.length` | | **initialize()** | Copies `nextDkgCeremony` from V1, sets `initialized = true` | | **Active validators** | `addedAtHeight = 0`, `deletedAtHeight = 0` | | **Inactive validators** | `addedAtHeight = deletedAtHeight = block.timestamp` | | **Filter logic** | `v.addedAtHeight <= H && (deletedAtHeight == 0 \|\| deletedAtHeight > H)` | ## Migration Steps (Existing Networks) 1. Release new node software with Allegro hardfork support 2. Schedule the fork by updating chainspec with target `allegroTime` 3. At fork activation: CL reads from V1 (since `isInitialized() == false`) 4. Admin migrates validators: `migrateValidator(0)`, `migrateValidator(1)`, ... 5. Admin calls `initialize()` → sets `initialized = true`, CL reads from V2 6. Post-migration: All reads/writes use V2 state directly ## New Networks 1. Initialize V2 at genesis with the initial validator set (set `initialized = true`) 2. Set `allegroTime = 0` to activate V2 immediately 3. V1 Validator Config contract/precompile is not necessary --------- Co-authored-by: Amp <amp@ampcode.com> Co-authored-by: Richard Janis Goldschmidt <github@aberrat.io> Co-authored-by: Tanishk Goyal <goyaltanishk02@gmail.com>
8c6d695 to
5160339
Compare
| function deactivateValidator(address validatorAddress) external; | ||
|
|
||
| /// @notice Rotate a validator to a new identity (owner or validator only) | ||
| /// @dev Atomically deletes the specified validator entry and adds a new one. This is equivalent |
There was a problem hiding this comment.
If you do it this way, and I rotate a validator mid-epoch, i would be ineligible to participate in the next DKG right? is that intentional? the fact that you're saying it is equivalent to delete and then add suggests that it is intentional, but given our current thin margins is this safe (we might have too few players available for the next dkg)
There was a problem hiding this comment.
nevermind now i understand. it might be worth explaining to validators that rotation involves a one epoch worth of deactivation (if it wasn't explained somewhere that i missed) but i think no change needed here.
There was a problem hiding this comment.
maybe worth a test case?
There was a problem hiding this comment.
It is briefly explained in the doc, but I agree - we should make this part of the docs so people understand.
But you pointing this out raises a point about how this spec wants to make deactivation immediate while leaving 1 epoch to warmup the new validator:
- in epoch
E: callrotateValidator($old, $new) - in epoch
E+1:$oldleaves the set of players (it is still a validator + dealer in this epoch);$newis not a player, merely registered as a peer and receives votes and certificates allowing it to sync up - in epoch
E+2:$olddropped out of the committee;$newis a player and will receives shares from the other dealers to acknowledge - in epoch
E+3:$newis in the committee.
This means there is a forced downtime of 1 epoch.
| 14. **Address update**: `transferValidatorOwnership` updates the address of an existing | ||
| validator without changing any other fields. This is useful for updating | ||
| addresses post-migration or correcting address assignments. | ||
|
|
There was a problem hiding this comment.
this clashes with invariant 2?
There was a problem hiding this comment.
Good catch! I updated invariant 2 and moved 14 into position 3 to clarify this point: validatorAddress is only mutable by the contract owner to address post-migration fixes. It cannot be changed by the owner of the entry.
| function deactivateValidator(address validatorAddress) external; | ||
|
|
||
| /// @notice Rotate a validator to a new identity (owner or validator only) | ||
| /// @dev Atomically deletes the specified validator entry and adds a new one. This is equivalent |
There was a problem hiding this comment.
maybe worth a test case?
| 8. **Historical consistency**: For any height H, the active validator set | ||
| consists of validators where `addedAtHeight <= H && (deactivatedAtHeight == 0 || | ||
| deactivatedAtHeight > H)`. Validators with `addedAtHeight == deactivatedAtHeight` are | ||
| never considered active. |
There was a problem hiding this comment.
should add !=0 here right? otherwise you rule out migrated vals...
There was a problem hiding this comment.
Thank you once more. This, too, was incorrect. addedAtHeight = block.height always: f80ecc4
| )) | ||
| ``` | ||
|
|
||
| The old validator is identified by `msg.sender`, which must be an existing active validator. |
There was a problem hiding this comment.
i think this contradicts invariant 11?
There was a problem hiding this comment.
Removed that comment - unnecessary and confusing: d57544c
|
|
||
| --- | ||
|
|
||
| # Invariants |
There was a problem hiding this comment.
Amp suggested this: "For validators added via addValidator, addedAtHeight > 0. addedAtHeight == 0 is reserved for validators migrated from V1
There was a problem hiding this comment.
I changed this in f80ecc4: addedAtHeight is now always set to block.height. While this might be confusing for historical backlook, this is fine from CL's point of view because we only care about looking forward.
…n/deletion of validators
| **Validator Addition and Deactivation**: When validators are added or deleted (this also applies to rotation), | ||
| there is no warmup period: deactivated validators are immediately removed from the set of players on the next epoch, | ||
| while activated validators are immediately added on the next epoch. This means that compared to validator config V1, | ||
| there is no cooldown and no warmup period. |
There was a problem hiding this comment.
@Zygimantass for confirmation: there is no cooldown and warmup period anymore.
Prepares peer management for the changes in #2181 (TIP 1017) by moving it to a self standing actor: after the hardfork, it will check every finalized block for changes to IP addresses in the peer set and update its tracked peers accordingly. For now there are no other changes other than delegating messages to the peer manager to its wrapped p2p oracle.
Prepares peer management for the changes in #2181 (TIP 1017) by moving it to a self standing actor: after the hardfork, it will check every finalized block for changes to IP addresses in the peer set and update its tracked peers accordingly. For now there are no other changes other than delegating messages to the peer manager to its wrapped p2p oracle.
… state (#2638) Migrates the DKG actor's state from a continuous journal to a `Metadata` object. The journal had unnecessary extra complexity that was not needed. The `Metadata` object is simpler to reason about. This migration is non-breaking: the journal is migrated to metadata and deleted. If a metadata object is already present it is not overwritten. As part of the migration, the DKG actor no longer tracks the socket addresses of dealers and players. This information is available in the execution layer state and can be read from there. These changes are in preparation for the Validator Config V2 where address tracking will be moved out of the DKG actor. Related #2181, #2617
Rendered TIP-1017