-
Notifications
You must be signed in to change notification settings - Fork 3
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
fix: pre approval flow fixes #65
Conversation
0652e9f
to
49640ac
Compare
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.
utACK
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.
Nice work! Some small nits, can ignore if you disagree
stakingOutputIndex uint32, | ||
stakingTime uint16, | ||
fpPubKeys []*btcec.PublicKey, | ||
pop *ProofOfPossession, |
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.
might be useful to check pop is not nil to avoid nil pointer
stakerdb/trackedtranactionstore.go
Outdated
return nil, fmt.Errorf("cannot add transaction without finality providers public keys") | ||
} | ||
|
||
var fpPubKeysBytes [][]byte = make([][]byte, len(fpPubKeys)) |
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.
var fpPubKeysBytes [][]byte = make([][]byte, len(fpPubKeys)) | |
fpPubKeysBytes := make([][]byte, len(fpPubKeys)) |
No need to be redundant with type declaration
stakerdb/trackedtranactionstore.go
Outdated
} | ||
|
||
return c.addTransactionInternal( | ||
txHashBytes, &msg, nil, |
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.
txHashBytes, &msg, nil, | |
txHash[:], &msg, nil, |
stakerdb/trackedtranactionstore.go
Outdated
serializedTx, err := utils.SerializeBtcTransaction(btcTx) | ||
|
||
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.
return err | |
return fmt.Errorf("failed to serialize Bitcoin transaction: %w", err) |
stakerdb/trackedtranactionstore.go
Outdated
update, err := newInitialUnbondingTxData(unbondingTx, unbondingTime) | ||
|
||
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.
return err | |
return fmt.Errorf("failed to create unbonding transaction data: %w", err) | |
CREATED
) to make sure staking command returns to caller only if staking tx was delivered either to Babylon or to BTC