-
Notifications
You must be signed in to change notification settings - Fork 29
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
Enforce uint64max weights #650
Conversation
@@ -346,7 +346,7 @@ func InitializeNativeTokenValidatorSet( | |||
validationIDs = append(validationIDs, subnetInfo.SubnetID.Append(uint32(i))) | |||
} | |||
|
|||
Expect(initialValidatorCreatedEvent.Weight).Should(Equal(new(big.Int).SetUint64(nodes[0].Weight))) | |||
Expect(initialValidatorCreatedEvent.Weight).Should(Equal(nodes[0].Weight)) |
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.
it's a nice side effect that these are now primitive types
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 love how much this code simplifies by making the go params ints!
Can we add documentation, stating that the reason we cap the total weight at a uint64
is because the p-chain does the same?
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.
LGTM
TBH, I think linking to the ACP-77 spec sufficiently documents this. I don't think it'd be helpful to explicitly document this data type here over the data type of any other field. Note that we do document how this limit affects staked value for the PoS case: https://github.com/ava-labs/teleporter/blob/main/contracts/validator-manager/README.md?plain=1#L72 |
Why this should be merged
Enforces that validator weights cannot exceed uint64max, which is the validator weight cap enforced on the P-Chain.
How this works
Adds an explicit check in
_initializeValidatorRegistration
. Note that the churn tracker also enforces this bound, but an explicit check is more readable and easier to test functionally.Also updates all variables/fields/params that represent weights to use
uint64
How this was tested
New unit test
How is this documented
N/A