-
Notifications
You must be signed in to change notification settings - Fork 10
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
chore: Validate pop cmd #118
Conversation
cmd/stakercli/pop/pop.go
Outdated
@@ -67,7 +75,7 @@ var generateCreatePopCmd = cli.Command{ | |||
cli.StringFlag{ | |||
Name: btcNetworkFlag, | |||
Usage: "Bitcoin network on which staking should take place (testnet3, mainnet, regtest, simnet, signet)", | |||
Value: "testnet3", | |||
Value: "main", |
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.
not a blocker, but imo commands by defaults should use testnets and act of acting on mainnet should be concious choice of the operator
cmd/stakercli/pop/pop.go
Outdated
|
||
secp256SigBase64, err := base64.StdEncoding.DecodeString(babySigOverBTCPk) | ||
if err != nil { | ||
return err |
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.
lets wrap the error
cmd/stakercli/pop/pop.go
Outdated
babySignBtcDoc := staker.NewCosmosSignDoc(babyAddr, base64Bytes) | ||
babySignBtcMarshaled, err := json.Marshal(babySignBtcDoc) | ||
if err != nil { | ||
return err |
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.
lets wrap the error
cmd/stakercli/pop/pop.go
Outdated
ArgsUsage: "<path-to-pop.json>", | ||
} | ||
|
||
type ValidationResult struct { |
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.
seems unused
cmd/stakercli/pop/pop.go
Outdated
|
||
bech32cosmosAddressString, err := sdk.Bech32ifyAddressBytes(babyPrefix, sdkAddress.Bytes()) | ||
if err != nil { | ||
return fmt.Errorf("failed to get babylon address bytes") |
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.
lets wrap error
|
||
var popsToVerify = []staker.Response{ | ||
{ | ||
BabyAddress: "bbn1xjz8fs9vkmefdqaxan5kv2d09vmwzru7jhy424", |
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.
just a question, as I assume this data was generated by generate pop command ?
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.
Ideally yes, but that would require some e2e tests to set up wallet, generate btc account, etc.
This PR:
generate-create-pop
output the result to a JSON filevalidate
pop cmd which takes the pop JSON file and verify pop