diff --git a/CHANGELOG.md b/CHANGELOG.md index 08536cca6..1bee57c27 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -46,6 +46,11 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) * [#169](https://github.com/babylonlabs-io/babylon/pull/169) Improve external events format and update events doc +### State Machine Breaking + +* [#185](https://github.com/babylonlabs-io/babylon/pull/185) Check that +unbonding / slashing transactions are standard + ## v0.12.0 ### State Machine Breaking diff --git a/btcstaking/staking.go b/btcstaking/staking.go index f6f682e03..8a04f7342 100644 --- a/btcstaking/staking.go +++ b/btcstaking/staking.go @@ -6,6 +6,7 @@ import ( "fmt" sdkmath "cosmossdk.io/math" + asig "github.com/babylonlabs-io/babylon/crypto/schnorr-adaptor-signature" "github.com/btcsuite/btcd/blockchain" "github.com/btcsuite/btcd/btcec/v2" "github.com/btcsuite/btcd/btcec/v2/schnorr" @@ -14,8 +15,15 @@ import ( "github.com/btcsuite/btcd/mempool" "github.com/btcsuite/btcd/txscript" "github.com/btcsuite/btcd/wire" +) - asig "github.com/babylonlabs-io/babylon/crypto/schnorr-adaptor-signature" +const ( + // MaxTxVersion is the maximum transaction version allowed in Babylon system. + // Changing that constant will require upgrade in the future, if we ever need + // to support v3 transactions. + MaxTxVersion = 2 + + MaxStandardTxWeight = 400000 ) // buildSlashingTxFromOutpoint builds a valid slashing transaction by creating a new Bitcoin transaction that slashes a portion @@ -215,6 +223,89 @@ func IsSimpleTransfer(tx *wire.MsgTx) error { return nil } +// CheckPreSignedTxSanity performs basic checks on a pre-signed transaction: +// - the transaction is not nil. +// - the transaction obeys basic BTC rules. +// - the transaction has exactly numInputs inputs. +// - the transaction has exactly numOutputs outputs. +// - the transaction lock time is 0. +// - the transaction version is between 1 and maxTxVersion. +// - each input has a sequence number equal to MaxTxInSequenceNum. +// - each input has an empty signature script. +// - each input has an empty witness. +func CheckPreSignedTxSanity( + tx *wire.MsgTx, + numInputs, numOutputs uint32, + maxTxVersion int32, +) error { + if tx == nil { + return fmt.Errorf("tx must not be nil") + } + + transaction := btcutil.NewTx(tx) + + if err := blockchain.CheckTransactionSanity(transaction); err != nil { + return fmt.Errorf("btc transaction do not obey BTC rules: %w", err) + } + + if len(tx.TxIn) != int(numInputs) { + return fmt.Errorf("tx must have exactly %d inputs", numInputs) + } + + if len(tx.TxOut) != int(numOutputs) { + return fmt.Errorf("tx must have exactly %d outputs", numOutputs) + } + + // this requirement makes every pre-signed tx final + if tx.LockTime != 0 { + return fmt.Errorf("pre-signed tx must not have locktime") + } + + if tx.Version > maxTxVersion || tx.Version < 1 { + return fmt.Errorf("tx version must be between 1 and %d", maxTxVersion) + } + + txWeight := blockchain.GetTransactionWeight(transaction) + + // Check that the transaction weight does not exceed the maximum standard tx weight + // alternative would be to require len(in.Witness) == 0 for all inptus. + if txWeight > MaxStandardTxWeight { + return fmt.Errorf("tx weight must not exceed %d", MaxStandardTxWeight) + } + + for _, in := range tx.TxIn { + if in.Sequence != wire.MaxTxInSequenceNum { + return fmt.Errorf("pre-signed tx must not be replaceable") + } + + // We require this to be 0, as all babylon pre-signed transactions use + // witness + if len(in.SignatureScript) != 0 { + return fmt.Errorf("pre-signed tx must not have signature script") + } + } + + return nil +} + +func CheckPreSignedUnbondingTxSanity(tx *wire.MsgTx) error { + return CheckPreSignedTxSanity( + tx, + 1, + 1, + MaxTxVersion, + ) +} + +func CheckPreSignedSlashingTxSanity(tx *wire.MsgTx) error { + return CheckPreSignedTxSanity( + tx, + 1, + 2, + MaxTxVersion, + ) +} + // validateSlashingTx performs basic checks on a slashing transaction: // - the slashing transaction is not nil. // - the slashing transaction has exactly one input. @@ -235,29 +326,9 @@ func validateSlashingTx( slashingChangeLockTime uint16, net *chaincfg.Params, ) error { - // Verify that the slashing transaction is not nil. - if slashingTx == nil { - return fmt.Errorf("slashing transaction must not be nil") - } - // Verify that the slashing transaction has exactly one input. - if len(slashingTx.TxIn) != 1 { - return fmt.Errorf("slashing transaction must have exactly one input") - } - - // Verify that the slashing transaction is non-replaceable. - if slashingTx.TxIn[0].Sequence != wire.MaxTxInSequenceNum { - return fmt.Errorf("slashing transaction must not be replaceable") - } - - // Verify that lock time of the slashing transaction is 0. - if slashingTx.LockTime != 0 { - return fmt.Errorf("slashing tx must not have locktime") - } - - // Verify that the slashing transaction has exactly two outputs. - if len(slashingTx.TxOut) != 2 { - return fmt.Errorf("slashing transaction must have exactly 2 outputs") + if err := CheckPreSignedSlashingTxSanity(slashingTx); err != nil { + return fmt.Errorf("invalid slashing tx: %w", err) } // Verify that at least staking output value * slashing rate is slashed. @@ -324,13 +395,13 @@ func validateSlashingTx( return nil } -// CheckTransactions validates all relevant data of slashing and funding transaction. +// CheckSlashingTxMatchFundingTx validates all relevant data of slashing and funding transaction. // - both transactions are valid from pov of BTC rules // - funding transaction has output committing to the provided script // - slashing transaction is valid // - slashing transaction input hash is pointing to funding transaction hash // - slashing transaction input index is pointing to funding transaction output committing to the script -func CheckTransactions( +func CheckSlashingTxMatchFundingTx( slashingTx *wire.MsgTx, fundingTransaction *wire.MsgTx, fundingOutputIdx uint32, @@ -345,10 +416,6 @@ func CheckTransactions( return fmt.Errorf("slashing and funding transactions must not be nil") } - if err := blockchain.CheckTransactionSanity(btcutil.NewTx(slashingTx)); err != nil { - return fmt.Errorf("slashing transaction does not obey BTC rules: %w", err) - } - if err := blockchain.CheckTransactionSanity(btcutil.NewTx(fundingTransaction)); err != nil { return fmt.Errorf("funding transaction does not obey BTC rules: %w", err) } diff --git a/btcstaking/staking_test.go b/btcstaking/staking_test.go index f1a180038..d8c7f0745 100644 --- a/btcstaking/staking_test.go +++ b/btcstaking/staking_test.go @@ -193,7 +193,7 @@ func testSlashingTx( require.ErrorIs(t, err, btcstaking.ErrDustOutputFound) } else { require.NoError(t, err) - err = btcstaking.CheckTransactions( + err = btcstaking.CheckSlashingTxMatchFundingTx( slashingTx, stakingTx, uint32(stakingOutputIdx), @@ -303,7 +303,7 @@ func TestSlashingTxWithOverflowMustNotAccepted(t *testing.T) { slashingTx.TxOut[0].Value = math.MaxInt64 / 8 slashingTx.TxOut[1].Value = math.MaxInt64 / 8 - err = btcstaking.CheckTransactions( + err = btcstaking.CheckSlashingTxMatchFundingTx( slashingTx, stakingTx, uint32(0), @@ -315,7 +315,7 @@ func TestSlashingTxWithOverflowMustNotAccepted(t *testing.T) { &chaincfg.MainNetParams, ) require.Error(t, err) - require.EqualError(t, err, "slashing transaction does not obey BTC rules: transaction output value is higher than max allowed value: 1152921504606846975 > 2.1e+15 ") + require.EqualError(t, err, "invalid slashing tx: btc transaction do not obey BTC rules: transaction output value is higher than max allowed value: 1152921504606846975 > 2.1e+15 ") } func TestNotAllowStakerKeyToBeFinalityProviderKey(t *testing.T) { @@ -413,3 +413,150 @@ func TestNotAllowFinalityProviderKeysAsCovenantKeys(t *testing.T) { require.Error(t, err) require.True(t, errors.Is(err, btcstaking.ErrDuplicatedKeyInScript)) } + +func TestCheckPreSignedTxSanity(t *testing.T) { + t.Parallel() + tests := []struct { + name string + genTx func() *wire.MsgTx + numInputs uint32 + numOutputs uint32 + maxTxVersion int32 + wantErr bool + expectedErrMsg string + }{ + { + name: "valid tx", + genTx: func() *wire.MsgTx { + tx := wire.NewMsgTx(2) + tx.AddTxIn(wire.NewTxIn(wire.NewOutPoint(&chainhash.Hash{}, 0), nil, nil)) + tx.AddTxOut(wire.NewTxOut(1000, nil)) + return tx + }, + numInputs: 1, + numOutputs: 1, + maxTxVersion: 2, + wantErr: false, + }, + { + name: "non standard version tx", + genTx: func() *wire.MsgTx { + tx := wire.NewMsgTx(0) + tx.AddTxIn(wire.NewTxIn(wire.NewOutPoint(&chainhash.Hash{}, 0), nil, nil)) + tx.AddTxOut(wire.NewTxOut(1000, nil)) + return tx + }, + numInputs: 1, + numOutputs: 1, + maxTxVersion: 2, + wantErr: true, + expectedErrMsg: "tx version must be between 1 and 2", + }, + { + name: "transaction with locktime", + genTx: func() *wire.MsgTx { + tx := wire.NewMsgTx(2) + tx.AddTxIn(wire.NewTxIn(wire.NewOutPoint(&chainhash.Hash{}, 0), nil, nil)) + tx.AddTxOut(wire.NewTxOut(1000, nil)) + tx.LockTime = 1 + return tx + }, + numInputs: 1, + numOutputs: 1, + maxTxVersion: 2, + wantErr: true, + expectedErrMsg: "pre-signed tx must not have locktime", + }, + { + name: "transaction with sig script", + genTx: func() *wire.MsgTx { + tx := wire.NewMsgTx(2) + tx.AddTxIn(wire.NewTxIn(wire.NewOutPoint(&chainhash.Hash{}, 0), nil, nil)) + tx.AddTxOut(wire.NewTxOut(1000, nil)) + tx.TxIn[0].SignatureScript = []byte{0x01, 0x02, 0x03} + return tx + }, + numInputs: 1, + numOutputs: 1, + maxTxVersion: 2, + wantErr: true, + expectedErrMsg: "pre-signed tx must not have signature script", + }, + { + name: "transaction with invalid amount of inputs", + genTx: func() *wire.MsgTx { + tx := wire.NewMsgTx(2) + tx.AddTxIn(wire.NewTxIn(wire.NewOutPoint(&chainhash.Hash{}, 0), nil, nil)) + tx.AddTxIn(wire.NewTxIn(wire.NewOutPoint(&chainhash.Hash{}, 1), nil, nil)) + tx.AddTxOut(wire.NewTxOut(1000, nil)) + return tx + }, + numInputs: 1, + numOutputs: 1, + maxTxVersion: 2, + wantErr: true, + expectedErrMsg: "tx must have exactly 1 inputs", + }, + { + name: "transaction with invalid amount of outputs", + genTx: func() *wire.MsgTx { + tx := wire.NewMsgTx(2) + tx.AddTxIn(wire.NewTxIn(wire.NewOutPoint(&chainhash.Hash{}, 0), nil, nil)) + tx.AddTxOut(wire.NewTxOut(1000, nil)) + tx.AddTxOut(wire.NewTxOut(1000, nil)) + return tx + }, + numInputs: 1, + numOutputs: 1, + maxTxVersion: 2, + wantErr: true, + expectedErrMsg: "tx must have exactly 1 outputs", + }, + { + name: "replacable transaction", + genTx: func() *wire.MsgTx { + tx := wire.NewMsgTx(2) + tx.AddTxIn(wire.NewTxIn(wire.NewOutPoint(&chainhash.Hash{}, 0), nil, nil)) + tx.AddTxOut(wire.NewTxOut(1000, nil)) + tx.TxIn[0].Sequence = wire.MaxTxInSequenceNum - 1 + return tx + }, + numInputs: 1, + numOutputs: 1, + maxTxVersion: 2, + wantErr: true, + expectedErrMsg: "pre-signed tx must not be replaceable", + }, + { + name: "transaction with too big witness", + genTx: func() *wire.MsgTx { + tx := wire.NewMsgTx(2) + tx.AddTxIn(wire.NewTxIn(wire.NewOutPoint(&chainhash.Hash{}, 0), nil, nil)) + tx.AddTxOut(wire.NewTxOut(1000, nil)) + witness := [20000000]byte{} + tx.TxIn[0].Witness = [][]byte{witness[:]} + return tx + }, + numInputs: 1, + numOutputs: 1, + maxTxVersion: 2, + wantErr: true, + expectedErrMsg: "tx weight must not exceed 400000", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + err := btcstaking.CheckPreSignedTxSanity( + tt.genTx(), tt.numInputs, tt.numOutputs, tt.maxTxVersion, + ) + + if tt.wantErr { + require.Error(t, err) + require.Contains(t, err.Error(), tt.expectedErrMsg) + } else { + require.NoError(t, err) + } + }) + } +} diff --git a/x/btcstaking/types/create_delegation_parser.go b/x/btcstaking/types/create_delegation_parser.go index 4d18f18a5..a00fa5640 100644 --- a/x/btcstaking/types/create_delegation_parser.go +++ b/x/btcstaking/types/create_delegation_parser.go @@ -10,7 +10,6 @@ import ( "github.com/btcsuite/btcd/wire" sdk "github.com/cosmos/cosmos-sdk/types" - "github.com/babylonlabs-io/babylon/btcstaking" bbn "github.com/babylonlabs-io/babylon/types" ) @@ -177,10 +176,6 @@ func ParseCreateDelegationMessage(msg *MsgCreateBTCDelegation) (*ParsedCreateDel return nil, fmt.Errorf("failed to deserialize unbonding tx: %v", err) } - if err := btcstaking.IsSimpleTransfer(unbondingTx.Transaction); err != nil { - return nil, fmt.Errorf("unbonding tx is not a simple transfer: %v", err) - } - unbondingSlashingTx, err := NewBtcTransaction(msg.UnbondingSlashingTx.MustMarshal()) if err != nil { diff --git a/x/btcstaking/types/validate_parsed_message.go b/x/btcstaking/types/validate_parsed_message.go index f6f7e40ca..139ca0204 100644 --- a/x/btcstaking/types/validate_parsed_message.go +++ b/x/btcstaking/types/validate_parsed_message.go @@ -78,7 +78,7 @@ func ValidateParsedMessageAgainstTheParams( ) } - if err := btcstaking.CheckTransactions( + if err := btcstaking.CheckSlashingTxMatchFundingTx( pm.StakingSlashingTx.Transaction, pm.StakingTx.Transaction, stakingOutputIdx, @@ -108,9 +108,16 @@ func ValidateParsedMessageAgainstTheParams( } // 3. Validate all data related to unbonding tx: + // - it is valid BTC pre-signed transaction // - it has valid unbonding output // - slashing tx is relevant to unbonding tx // - slashing tx signature is valid + if err := btcstaking.CheckPreSignedUnbondingTxSanity( + pm.UnbondingTx.Transaction, + ); err != nil { + return nil, ErrInvalidUnbondingTx.Wrapf("unbonding tx is not a valid pre-signed transaction: %v", err) + } + unbondingInfo, err := btcstaking.BuildUnbondingInfo( pm.StakerPK.PublicKey, pm.FinalityProviderKeys.PublicKeys, @@ -129,7 +136,7 @@ func ValidateParsedMessageAgainstTheParams( return nil, ErrInvalidUnbondingTx.Wrapf("unbonding tx does not contain expected unbonding output") } - err = btcstaking.CheckTransactions( + err = btcstaking.CheckSlashingTxMatchFundingTx( pm.UnbondingSlashingTx.Transaction, pm.UnbondingTx.Transaction, unbondingOutputIdx,