-
Notifications
You must be signed in to change notification settings - Fork 680
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ACP-77: Implement validator state #3388
Changes from 222 commits
9c8835b
733d5fd
10d8e86
a2f69d0
ee287c6
a42e48c
4c542af
941f536
9161864
dfded5e
f275c15
d9355cc
46501ad
b7459bd
b81b737
f6bb383
7a6f7eb
64dec9c
33eb562
cba7a50
b4955d6
3c9096e
1b22842
ad83805
7820c17
20ed013
fcb388c
a02b51c
daf45a3
7bda9b1
f24ca9f
463ce02
4ce1977
3cc413a
4de908e
09f71e6
114beeb
0cd4f89
57810a0
e89671a
72f972c
e8eea6f
d67408d
8571163
6e457fa
86754d0
c68afec
09faa6b
51937f7
88a310e
b11ff7e
6c9e0e0
b028ada
42d61ef
8ef4fcf
e2045e2
863c92e
e0beab1
410bcda
149e21e
6c0945b
a441d62
fbc5ce1
cfc2790
449994a
744f4c7
d9108a7
98020f1
cc9a086
a3f0d94
de81518
c062890
b68ed8a
8fe987c
30efc6d
903f7c3
34fe3a6
b9d5507
99f0cde
00fc8d1
c08a1d7
9d07a45
f91e0a8
a35b090
dd7029c
8978452
a383209
8b52ead
3859d98
0462fda
5a362bd
a3cc8e8
ddb9ba6
556f1eb
891bca4
a33e0df
8fee2b4
0d3d3b9
01c9072
01de954
1ca7524
237f590
bc53046
127a3fa
1986804
8279972
3d5d6c1
d7ba4fb
d550c97
9226649
83cc0dc
702639a
4b562f0
7f3dd5e
f40c1b3
f69ec51
51f009b
36714ed
05a7b13
09c5126
a1db89e
2e7595c
575b36a
d2d0f69
b6b6515
6ffcd1b
ad00180
f1b07d2
7fb99c0
65b5d50
8791e78
f31cfc6
fe9a76d
58d5b8d
273fbbe
290ef97
9155e1f
cadf8f7
803d0c4
76f4eee
a2c7773
95c42a1
3d7bd81
bbc395b
3a03597
21cdc32
845f1d2
91a7cd7
df02f3c
c848fee
79c9b40
d8819fc
667002e
31be1e2
8f0a080
d482de8
e8b21ec
7f517c7
7d0d7e5
08bd9e3
78c1c3d
6547c6d
38ee164
d2137ef
5845d11
6c3116a
d0d1602
d397375
5f8a09c
3e9dc01
da3a726
8483ced
0507ce7
0022a65
8bbeee6
08dd776
255b0bf
3bc547d
bff468d
99f3c97
7cf1668
41f78f0
29cd6ba
8dfcbb1
ef29548
a77fb3c
ce05dc8
3d04cef
dc35645
d472a9f
34ba29b
97029aa
dbeee70
729ded5
3621e53
de2be9f
92a2277
6375aa2
aedff15
1ac030a
9e0d7d5
9993b05
547d426
6bcc0ea
6065604
a7792c7
39b961b
4723c46
46c4889
a576cd1
f1ca6e6
71f88e8
c2ffd17
66011f0
32bba0e
d183148
b1bb458
ad31107
cee236e
900eba3
fd48bde
7cbf31b
3077356
69837c1
d37e9f3
9dc642a
22de2b1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -35,15 +35,17 @@ type diff struct { | |
parentID ids.ID | ||
stateVersions Versions | ||
|
||
timestamp time.Time | ||
feeState gas.State | ||
sovExcess gas.Gas | ||
accruedFees uint64 | ||
timestamp time.Time | ||
feeState gas.State | ||
sovExcess gas.Gas | ||
accruedFees uint64 | ||
parentActiveSOVs int | ||
|
||
// Subnet ID --> supply of native asset of the subnet | ||
currentSupply map[ids.ID]uint64 | ||
|
||
expiryDiff *expiryDiff | ||
sovDiff *subnetOnlyValidatorsDiff | ||
|
||
currentStakerDiffs diffStakers | ||
// map of subnetID -> nodeID -> total accrued delegatee rewards | ||
|
@@ -83,7 +85,9 @@ func NewDiff( | |
feeState: parentState.GetFeeState(), | ||
sovExcess: parentState.GetSoVExcess(), | ||
accruedFees: parentState.GetAccruedFees(), | ||
parentActiveSOVs: parentState.NumActiveSubnetOnlyValidators(), | ||
expiryDiff: newExpiryDiff(), | ||
sovDiff: newSubnetOnlyValidatorsDiff(), | ||
subnetOwners: make(map[ids.ID]fx.Owner), | ||
subnetConversions: make(map[ids.ID]SubnetConversion), | ||
}, nil | ||
|
@@ -194,6 +198,70 @@ func (d *diff) DeleteExpiry(entry ExpiryEntry) { | |
d.expiryDiff.DeleteExpiry(entry) | ||
} | ||
|
||
func (d *diff) GetActiveSubnetOnlyValidatorsIterator() (iterator.Iterator[SubnetOnlyValidator], error) { | ||
parentState, ok := d.stateVersions.GetState(d.parentID) | ||
if !ok { | ||
return nil, fmt.Errorf("%w: %s", ErrMissingParentState, d.parentID) | ||
} | ||
|
||
parentIterator, err := parentState.GetActiveSubnetOnlyValidatorsIterator() | ||
if err != nil { | ||
return nil, err | ||
} | ||
|
||
return d.sovDiff.getActiveSubnetOnlyValidatorsIterator(parentIterator), nil | ||
} | ||
|
||
func (d *diff) NumActiveSubnetOnlyValidators() int { | ||
return d.parentActiveSOVs + d.sovDiff.numAddedActive | ||
} | ||
|
||
func (d *diff) WeightOfSubnetOnlyValidators(subnetID ids.ID) (uint64, error) { | ||
if weight, modified := d.sovDiff.modifiedTotalWeight[subnetID]; modified { | ||
return weight, nil | ||
} | ||
|
||
parentState, ok := d.stateVersions.GetState(d.parentID) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can you explain the high level idea of doing this backwards traversal in search of the subnet, both here and in the next function? We basically don't have a full snapshot of the validators in each state? Why is that? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Performing a full copy of the validator states for each diff would be pretty expensive. The number of changes per block is typically small, but the number total entries being tracked can be quite large. |
||
if !ok { | ||
return 0, fmt.Errorf("%w: %s", ErrMissingParentState, d.parentID) | ||
} | ||
|
||
return parentState.WeightOfSubnetOnlyValidators(subnetID) | ||
} | ||
|
||
func (d *diff) GetSubnetOnlyValidator(validationID ids.ID) (SubnetOnlyValidator, error) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A couple high level questions that may be related:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Diffs are created for a few reasons. But the reason the diffs are designed this way is because each diff (can) represent the changes that are performed by a block. When a block is verified a diff is created. When a block is accepted a diff is applied to the disk state. Diffs are updated in-place during the execution (verification) of a block - but we wouldn't want a child block to modify the state that will be committed when the parent block is accepted (as the child could be rejected for a different child) |
||
if sov, modified := d.sovDiff.modified[validationID]; modified { | ||
if sov.isDeleted() { | ||
return SubnetOnlyValidator{}, database.ErrNotFound | ||
} | ||
return sov, nil | ||
} | ||
|
||
parentState, ok := d.stateVersions.GetState(d.parentID) | ||
if !ok { | ||
return SubnetOnlyValidator{}, fmt.Errorf("%w: %s", ErrMissingParentState, d.parentID) | ||
} | ||
|
||
return parentState.GetSubnetOnlyValidator(validationID) | ||
} | ||
|
||
func (d *diff) HasSubnetOnlyValidator(subnetID ids.ID, nodeID ids.NodeID) (bool, error) { | ||
if has, modified := d.sovDiff.hasSubnetOnlyValidator(subnetID, nodeID); modified { | ||
return has, nil | ||
} | ||
|
||
parentState, ok := d.stateVersions.GetState(d.parentID) | ||
if !ok { | ||
return false, fmt.Errorf("%w: %s", ErrMissingParentState, d.parentID) | ||
} | ||
|
||
return parentState.HasSubnetOnlyValidator(subnetID, nodeID) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this might seem as a weird question, but - how deep is the expected recursion here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The recursion ends at the lastAcceptedState and starts at the most recently verified block, so typically the recursion is very shallow. |
||
} | ||
|
||
func (d *diff) PutSubnetOnlyValidator(sov SubnetOnlyValidator) error { | ||
return d.sovDiff.putSubnetOnlyValidator(d, sov) | ||
} | ||
|
||
func (d *diff) GetCurrentValidator(subnetID ids.ID, nodeID ids.NodeID) (*Staker, error) { | ||
// If the validator was modified in this diff, return the modified | ||
// validator. | ||
|
@@ -504,6 +572,26 @@ func (d *diff) Apply(baseState Chain) error { | |
baseState.DeleteExpiry(entry) | ||
} | ||
} | ||
// Ensure that all sov deletions happen before any sov additions. This | ||
// ensures that a subnetID+nodeID pair that was deleted and then re-added in | ||
// a single diff can't get reordered into the addition happening first; | ||
// which would return an error. | ||
michaelkaplan13 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
for _, sov := range d.sovDiff.modified { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Playing devil's advocate here, we're iterating here over a map, and calling Is that a concern? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is generally unexpected for
|
||
if !sov.isDeleted() { | ||
continue | ||
} | ||
if err := baseState.PutSubnetOnlyValidator(sov); err != nil { | ||
return err | ||
} | ||
} | ||
for _, sov := range d.sovDiff.modified { | ||
if sov.isDeleted() { | ||
continue | ||
} | ||
if err := baseState.PutSubnetOnlyValidator(sov); err != nil { | ||
return err | ||
} | ||
} | ||
for _, subnetValidatorDiffs := range d.currentStakerDiffs.validatorDiffs { | ||
for _, validatorDiff := range subnetValidatorDiffs { | ||
switch validatorDiff.validatorStatus { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: numParentActiveSOVs is more descriptive IMO
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or
parentActiveSOVCount
because a "count" is more descriptive than a "number" 😉There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I ended up going with
parentNumActiveSOVs
.Because it aligns most with the actual source of this:
parentState.NumActiveSubnetOnlyValidators()