-
Notifications
You must be signed in to change notification settings - Fork 79
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
Use local machine to join as validator on any L1 #2548
base: main
Are you sure you want to change the base?
Conversation
Can you elaborate in the PR description how a user would accomplish this with the CLI? Is there a new command to import a new L1 configuration or add an active RPC endpoint as a known L1 network? How does this work exactly |
ec0a639
to
830b1d2
Compare
@@ -1563,60 +1519,6 @@ func ConvertURIToPeers(uris []string) ([]info.Peer, error) { | |||
return aggregatorPeers, nil | |||
} | |||
|
|||
func GetAggregatorExtraPeers( |
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.
Moving these functions to blockchain pkg so that it can be re used across cmds
@@ -289,46 +289,3 @@ func getThreshold(maxLen int) (uint32, error) { | |||
} | |||
return uint32(intTh), err | |||
} | |||
|
|||
func getKeyForChangeOwner(network models.Network) (string, error) { |
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.
Moving these functions to blockchain pkg so that it can be re used across cmds
@@ -390,7 +390,7 @@ func InitValidatorRegistration( | |||
ux.Logger.PrintLineSeparator() | |||
ux.Logger.PrintToUser("Initializing a validator registration with PoS validator manager") | |||
ux.Logger.PrintToUser("Using rpcURL: %s", rpcURL) | |||
ux.Logger.PrintToUser("NodeID: %s staking %s for %ds", nodeID.String(), stakeAmount, uint64(stakeDuration.Seconds())) | |||
ux.Logger.PrintToUser("NodeID: %s staking %s", nodeID.String(), stakeAmount) |
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.
We don't print duration for L1 validation
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.
isn't this part of PoS staking?
aggregatorLogger, | ||
true, | ||
delegationFee, | ||
genesis.FujiParams.MinStakeDuration, |
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.
Duration is not needed for POS staking, choosing to use genesis.FujiParams.MinStakeDuration as a placeholder duration
pkg/blockchain/helper.go
Outdated
return peers, nil | ||
} | ||
|
||
func GetBLSInfo(publicKey, proofOfPossesion string) (signer.ProofOfPossession, error) { |
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.
change the name to something like ToProofOfProssession
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.
addressed
@@ -444,9 +444,11 @@ func InitValidatorRegistration( | |||
ux.Logger.PrintToUser("Error getting validator weight") | |||
return nil, ids.Empty, err | |||
} | |||
} else { | |||
// Print validator weight for POA only | |||
ux.Logger.PrintToUser(fmt.Sprintf("Validator weight: %d", 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.
can we check if PChain weight is also important for PoS? It seems so IMO
@@ -390,7 +390,7 @@ func InitValidatorRegistration( | |||
ux.Logger.PrintLineSeparator() | |||
ux.Logger.PrintToUser("Initializing a validator registration with PoS validator manager") | |||
ux.Logger.PrintToUser("Using rpcURL: %s", rpcURL) | |||
ux.Logger.PrintToUser("NodeID: %s staking %s for %ds", nodeID.String(), stakeAmount, uint64(stakeDuration.Seconds())) | |||
ux.Logger.PrintToUser("NodeID: %s staking %s", nodeID.String(), stakeAmount) |
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.
isn't this part of PoS staking?
func newLocalValidateCmd() *cobra.Command { | ||
cmd := &cobra.Command{ | ||
Use: "validate [clusterName]", | ||
Short: "Validate a specified L1 with an Avalanche Node set up on local machine (PoS only)", |
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 don't believe it is PoS only. Indeed current impl seems to be PoA only
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.
for POA only validatormanager privatekey owner can add validator. For local machine, if it wants to join an L1, the l1 has to be POS.
Long: `Use Avalanche Node set up on local machine to set up specified L1 by providing the | ||
RPC URL of the L1. | ||
|
||
This command can only be used to validate Proof of Stake L1.`, |
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.
Does not serve for PoA?
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.
see above
return err | ||
} | ||
} | ||
_, blockchainID, err := utils.SplitAvalanchegoRPCURI(rpcURL) |
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.
this assumes too much on the rpc url.... better to either get blockchain ID from chain spec query contract.GetBlockchainID,
and, if not found, prompt the user
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.
RPC url will always have blockchain id in it?
cmd/nodecmd/local.go
Outdated
cmd.Flags().StringVar(&blockchainName, "blockchain", "", "specify the blockchain the node is syncing with") | ||
cmd.Flags().Uint64Var(&stakeAmount, "stake-amount", 0, "(PoS only) amount of tokens to stake") | ||
cmd.Flags().StringVar(&rpcURL, "rpc", "", "connect to validator manager at the given rpc endpoint") | ||
cmd.Flags().Uint64Var(&balance, "balance", 0, "set the AVAX balance of the validator that will be used for continuous fee on P-Chain") |
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.
can you use float balance as in other commands?
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.
addressed
cmd/nodecmd/local.go
Outdated
return err | ||
} | ||
} else { | ||
// convert to nanoAVAX |
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.
this has changed, please check latest main on how to best do this for float avax balance
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.
addressed
balance *= units.Avax | ||
} | ||
|
||
if remainingBalanceOwnerAddr == "" { |
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.
need flag for this
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.
addressed
Addresses: remainingBalanceOwnerAddrID, | ||
} | ||
|
||
if disableOwnerAddr == "" { |
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.
need flag for this
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.
addressed
Signed-off-by: sukantoraymond <[email protected]>
This PR enables using local machine to join any L1 (Proof of Stake) as a validator.
To join an L1 as validator, call
avalanche node local start <clusterName>
followed byavalanche node local validate <clusterName>
. User will be prompted for RPC URL.If has more than 1 node in it, all nodes will be added as validators of the specified L1
E2E test will be implemented once #2528 is resolved