-
Notifications
You must be signed in to change notification settings - Fork 40
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
Properly validate whether tranactions are standard #185
Properly validate whether tranactions are standard #185
Conversation
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
@@ -413,3 +413,148 @@ func TestNotAllowFinalityProviderKeysAsCovenantKeys(t *testing.T) { | |||
require.Error(t, err) | |||
require.True(t, errors.Is(err, btcstaking.ErrDuplicatedKeyInScript)) | |||
} | |||
|
|||
func TestCheckPreSignedTxSanity(t *testing.T) { | |||
tests := []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.
t.Parallel()
} | ||
for _, tt := range tests { | ||
t.Run(tt.name, func(t *testing.T) { | ||
err := btcstaking.CheckPreSignedTxSanity( |
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.
t.Parallel() in subtest also
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 this need to be backported or it can go to the next version?
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.
Great work!
btcstaking/staking.go
Outdated
@@ -215,6 +223,89 @@ func IsSimpleTransfer(tx *wire.MsgTx) error { | |||
return nil | |||
} | |||
|
|||
// CheckPreSignedTxSanity perfomorms basic checks on a pre-signed transaction: |
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.
// CheckPreSignedTxSanity perfomorms basic checks on a pre-signed transaction: | |
// CheckPreSignedTxSanity performs basic checks on a pre-signed transaction: |
I wanted to backport it to release patch version v0.12.1, and bump phase-1 services which use this validation: staking-api, and covenant-emulator. Indexer does not need it as it reads data from BTC ledger, so if miner inserted v0 transaction into the ledger, then indexer must process it |
Add: - new `btcstaking` library function to validate whether transactions are standard based on https://github.com/btcsuite/btcd/blob/ee68dc66a835bf2c9333f3d7b33791841f561c84/mempool/policy.go#L285 - use those function to validate unbonding / slashing transactions - test suite to test new functions
Add: - new `btcstaking` library function to validate whether transactions are standard based on https://github.com/btcsuite/btcd/blob/ee68dc66a835bf2c9333f3d7b33791841f561c84/mempool/policy.go#L285 - use those function to validate unbonding / slashing transactions - test suite to test new functions
Add:
btcstaking
library function to validate whether transactions are standard based on https://github.com/btcsuite/btcd/blob/ee68dc66a835bf2c9333f3d7b33791841f561c84/mempool/policy.go#L285