Initial validator sets

[minghinmatthewlam]

Why this should be merged

Fixes #543 and #542

How this works

• Adds a separate initializeValidatorSet call which expects a Warp message that contains a SubnetConversionMessage in its payload, and then verifies the tx ID against the hash of the bytes for the passed in SubnetConversionData with the initial validators.
• Restricts initial valiator registration until initial validator set is set, and implicitly has checked for other calls to not be callable until initializedValidatorSet
• Removes owner from the base struct of Validator, any PoA validator either added from initial validator set, or from a PoA validator manager contract can be intiialized end by anyone after migration to PoS.
• For PoA, only owner can call intiialize registration/end for any validators, fixes a bug where after owner change the new owner doesn't have access.
• PoS validator manager contract keeps track of its added validations, so it knows which validations are PoS and eligibile for rewards, etc.
• Reentrancy guard is moved to only PoS, because PoA doesn't have external calls to lock/unlock stake, only calls to Warp.

How this was tested

TODO

How is this documented

comments

[geoff-vball]

I know this is still WIP but wanted to leave feedback as early as possible

[cam-schultz]

Generally makes sense to me. I left a handful of clarifying questions and cleanup suggestions.

[cam-schultz]

I think we should have the name here match the spec. The spec update calls this ValidatorData. Between the two, I prefer InitialValidator, unless there's another scenario in which ValidatorData would be used outside of initialization. @michaelkaplan13

[cam-schultz]

Is this check strictly necessary? IMO node ID validity should be checked on the P-Chain side, so if the P-Chain is willing to sign it we should accept it. A zero node ID would cause the hash comparison to fail.

[cam-schultz]

Expanding on this, one could argue all the validity checks on subnetConversionData are unnecessary since the subnetConversionID from the Warp message already encodes validated data (by the P-Chain). If the caller supplies invalid subnetConversionData here, the hash comparison will fail regardless, and the result would be the same as if we checks the input's validity.

[iansuvak]

I agree and would be fine removing all of these checks.

[cam-schultz]

I removed all validity checks on subnetConversionData, but note that we still need to check that the blockchainID and validator manager address retreived from the Warp message match the contract instance.

[bernard-avalabs]

Generally makes sense to me.