From 957168afb3f883193a0e8c0f6863a7d0fe121560 Mon Sep 17 00:00:00 2001 From: Tomasz Slabon Date: Wed, 13 Mar 2024 18:01:09 +0100 Subject: [PATCH 1/3] Added additional check when ensuring wallet synchronization --- pkg/chain/ethereum/tbtc.go | 13 ++++-- pkg/maintainer/spv/chain.go | 13 ++---- pkg/maintainer/spv/chain_test.go | 6 +-- pkg/maintainer/spv/moved_funds_sweep.go | 8 +++- pkg/tbtc/chain.go | 18 ++++---- pkg/tbtc/chain_test.go | 7 +++ pkg/tbtc/wallet.go | 60 ++++++++++++++++++++----- pkg/tbtcpg/chain.go | 7 --- pkg/tbtcpg/chain_test.go | 6 +-- pkg/tbtcpg/moved_funds_sweep.go | 8 +++- 10 files changed, 98 insertions(+), 48 deletions(-) diff --git a/pkg/chain/ethereum/tbtc.go b/pkg/chain/ethereum/tbtc.go index d5a1dc5c85..31f99f2e9d 100644 --- a/pkg/chain/ethereum/tbtc.go +++ b/pkg/chain/ethereum/tbtc.go @@ -2026,7 +2026,7 @@ func (tc *TbtcChain) GetMovingFundsParameters() ( func (tc *TbtcChain) GetMovedFundsSweepRequest( movingFundsTxHash bitcoin.Hash, movingFundsTxOutpointIndex uint32, -) (*tbtc.MovedFundsSweepRequest, error) { +) (*tbtc.MovedFundsSweepRequest, bool, error) { movedFundsKey := buildMovedFundsKey( movingFundsTxHash, movingFundsTxOutpointIndex, @@ -2036,16 +2036,21 @@ func (tc *TbtcChain) GetMovedFundsSweepRequest( movedFundsKey, ) if err != nil { - return nil, fmt.Errorf( + return nil, false, fmt.Errorf( "cannot get moved funds sweep request for key [0x%x]: [%v]", movedFundsKey.Text(16), err, ) } + // Moved funds sweep request not found. + if movedFundsSweepRequest.CreatedAt == 0 { + return nil, false, nil + } + state, err := parseMovedFundsSweepRequestState(movedFundsSweepRequest.State) if err != nil { - return nil, fmt.Errorf( + return nil, false, fmt.Errorf( "cannot parse state for moved funds sweep request [0x%x]: [%v]", movedFundsKey.Text(16), err, @@ -2057,7 +2062,7 @@ func (tc *TbtcChain) GetMovedFundsSweepRequest( Value: movedFundsSweepRequest.Value, CreatedAt: time.Unix(int64(movedFundsSweepRequest.CreatedAt), 0), State: state, - }, nil + }, true, nil } func parseMovedFundsSweepRequestState(value uint8) ( diff --git a/pkg/maintainer/spv/chain.go b/pkg/maintainer/spv/chain.go index 1c810bc494..c9e060e9bf 100644 --- a/pkg/maintainer/spv/chain.go +++ b/pkg/maintainer/spv/chain.go @@ -21,14 +21,8 @@ type Chain interface { ) error // GetDepositRequest gets the on-chain deposit request for the given - // funding transaction hash and output index.The returned values represent: - // - deposit request which is non-nil only when the deposit request was - // found, - // - boolean value which is true if the deposit request was found, false - // otherwise, - // - error which is non-nil only when the function execution failed. It will - // be nil if the deposit request was not found, but the function execution - // succeeded. + // funding transaction hash and output index. The returned bool value + // indicates whether the request was found or not. GetDepositRequest( fundingTxHash bitcoin.Hash, fundingOutputIndex uint32, @@ -59,10 +53,11 @@ type Chain interface { // GetMovedFundsSweepRequest gets the on-chain moved funds sweep request for // the given moving funds transaction hash and output index. + // The returned bool value indicates whether the request was found or not. GetMovedFundsSweepRequest( movingFundsTxHash bitcoin.Hash, movingFundsTxOutpointIndex uint32, - ) (*tbtc.MovedFundsSweepRequest, error) + ) (*tbtc.MovedFundsSweepRequest, bool, error) // SubmitRedemptionProofWithReimbursement submits the redemption proof // via MaintainerProxy. The caller is reimbursed. diff --git a/pkg/maintainer/spv/chain_test.go b/pkg/maintainer/spv/chain_test.go index 35eec53b89..2a0bcc0a89 100644 --- a/pkg/maintainer/spv/chain_test.go +++ b/pkg/maintainer/spv/chain_test.go @@ -666,7 +666,7 @@ func (lc *localChain) setMovedFundsSweepRequest( func (lc *localChain) GetMovedFundsSweepRequest( movingFundsTxHash bitcoin.Hash, movingFundsTxOutpointIndex uint32, -) (*tbtc.MovedFundsSweepRequest, error) { +) (*tbtc.MovedFundsSweepRequest, bool, error) { lc.mutex.Lock() defer lc.mutex.Unlock() @@ -677,10 +677,10 @@ func (lc *localChain) GetMovedFundsSweepRequest( request, ok := lc.movedFundsSweepRequests[requestKey] if !ok { - return nil, fmt.Errorf("request not found") + return nil, false, nil } - return request, nil + return request, true, nil } type mockBlockCounter struct { diff --git a/pkg/maintainer/spv/moved_funds_sweep.go b/pkg/maintainer/spv/moved_funds_sweep.go index e38c666d83..e45904890a 100644 --- a/pkg/maintainer/spv/moved_funds_sweep.go +++ b/pkg/maintainer/spv/moved_funds_sweep.go @@ -264,7 +264,7 @@ func isUnprovenMovedFundsSweepTransaction( requestTransactionHash := transaction.Inputs[0].Outpoint.TransactionHash requestOutputIndex := transaction.Inputs[0].Outpoint.OutputIndex - movedFundsSweepRequest, err := spvChain.GetMovedFundsSweepRequest( + movedFundsSweepRequest, isRequest, err := spvChain.GetMovedFundsSweepRequest( requestTransactionHash, requestOutputIndex, ) @@ -275,6 +275,12 @@ func isUnprovenMovedFundsSweepTransaction( ) } + // Check is it's a moved funds sweep request at all. + if !isRequest { + return false, nil + } + + // Check if it's a pending moved funds sweep request. if movedFundsSweepRequest.State != tbtc.MovedFundsStatePending { return false, nil } diff --git a/pkg/tbtc/chain.go b/pkg/tbtc/chain.go index dfd730acd3..977a7a744f 100644 --- a/pkg/tbtc/chain.go +++ b/pkg/tbtc/chain.go @@ -209,18 +209,20 @@ type BridgeChain interface { ) (*RedemptionRequest, bool, error) // GetDepositRequest gets the on-chain deposit request for the given - // funding transaction hash and output index.The returned values represent: - // - deposit request which is non-nil only when the deposit request was - // found, - // - boolean value which is true if the deposit request was found, false - // otherwise, - // - error which is non-nil only when the function execution failed. It will - // be nil if the deposit request was not found, but the function execution - // succeeded. + // funding transaction hash and output index. The returned bool value + // indicates whether the request was found or not. GetDepositRequest( fundingTxHash bitcoin.Hash, fundingOutputIndex uint32, ) (*DepositChainRequest, bool, error) + + // GetMovedFundsSweepRequest gets the on-chain moved funds sweep request for + // the given moving funds transaction hash and output index. + // The returned bool value indicates whether the request was found or not. + GetMovedFundsSweepRequest( + movingFundsTxHash bitcoin.Hash, + movingFundsTxOutpointIndex uint32, + ) (*MovedFundsSweepRequest, bool, error) } // NewWalletRegisteredEvent represents a new wallet registered event. diff --git a/pkg/tbtc/chain_test.go b/pkg/tbtc/chain_test.go index 90a076c02e..c505d41056 100644 --- a/pkg/tbtc/chain_test.go +++ b/pkg/tbtc/chain_test.go @@ -199,6 +199,13 @@ func (lc *localChain) IsBetaOperator() (bool, error) { panic("unsupported") } +func (lc *localChain) GetMovedFundsSweepRequest( + movingFundsTxHash bitcoin.Hash, + movingFundsTxOutpointIndex uint32, +) (*MovedFundsSweepRequest, bool, error) { + panic("unsupported") +} + func (lc *localChain) GetOperatorID( operatorAddress chain.Address, ) (chain.OperatorID, error) { diff --git a/pkg/tbtc/wallet.go b/pkg/tbtc/wallet.go index e5201a702a..f2c615808a 100644 --- a/pkg/tbtc/wallet.go +++ b/pkg/tbtc/wallet.go @@ -7,11 +7,12 @@ import ( "crypto/elliptic" "encoding/hex" "fmt" - "golang.org/x/exp/slices" "math/big" "sync" "time" + "golang.org/x/exp/slices" + "github.com/ipfs/go-log/v2" "github.com/keep-network/keep-core/pkg/bitcoin" "github.com/keep-network/keep-core/pkg/chain" @@ -579,8 +580,15 @@ func EnsureWalletSyncedBetweenChains( } for _, utxo := range allUtxos { - // We know that valid first transaction of the wallet always - // have just one output. Any utxos with output index other + // The first valid transaction of a wallet is most likely a deposit + // sweep, but there is a small chance it could be a moved funds sweep. + // It could happen if the wallet was selected as a target wallet and + // some funds were moved to it by the source wallet. In that case the + // wallet could create a moved fund sweep transaction even before + // sweeping any deposits. + + // In any case, we know that valid first transaction of the wallet + // always have just one output. Any utxos with output index other // than 0 are certainly not produced by the wallet and, we should // not take them into account. if utxo.Outpoint.OutputIndex != 0 { @@ -596,11 +604,10 @@ func EnsureWalletSyncedBetweenChains( ) } - // We know that valid first transaction of the wallet have all their - // inputs referring to revealed deposits. We need to check just - // one input. If it points to a revealed deposit, that means - // the given transaction is produced by our wallet. Otherwise, - // such a transaction is a spam. + // The transaction could be a deposit sweep. In that case all the + // transaction's inputs must refer to revealed deposits. We can + // check one input. If it points to a revealed deposit, that means + // the given transaction is produced by our wallet. input := transaction.Inputs[0] _, isDeposit, err := bridgeChain.GetDepositRequest( input.Outpoint.TransactionHash, @@ -617,11 +624,40 @@ func EnsureWalletSyncedBetweenChains( } if isDeposit { - // If that's the case, the wallet was already done their - // first Bitcoin transaction and the Bridge is awaiting the - // SPV proof. + // If that's the case, the wallet has already created a deposit + // sweep as their first Bitcoin transaction and the Bridge is + // awaiting the SPV proof. + return fmt.Errorf("wallet already produced their first " + + "Bitcoin transaction (deposit sweep); Bridge is probably " + + "awaiting the SPV proof", + ) + } + + // The transaction could be a moved funds sweep request. In that + // case the transaction's input must refer to a moved funds sweep + // request. If the input points to a moved funds sweep request, that + // means the given transaction is produced by our wallet. + _, isRequest, err := bridgeChain.GetMovedFundsSweepRequest( + input.Outpoint.TransactionHash, + input.Outpoint.OutputIndex, + ) + if err != nil { + return fmt.Errorf( + "cannot get moved funds sweep request for hash [%s] "+ + "and output index [%v]: [%v]", + input.Outpoint.TransactionHash.String(), + input.Outpoint.OutputIndex, + err, + ) + } + + if isRequest { + // If that's the case, the wallet has already created a moved + // funds sweep their first Bitcoin transaction and the Bridge + // is awaiting the SPV proof. return fmt.Errorf("wallet already produced their first " + - "Bitcoin transaction; Bridge is probably awaiting the SPV proof", + "Bitcoin transaction (moved funds sweep); Bridge is " + + "probably awaiting the SPV proof", ) } diff --git a/pkg/tbtcpg/chain.go b/pkg/tbtcpg/chain.go index c4b29d4a1d..6a85f121ab 100644 --- a/pkg/tbtcpg/chain.go +++ b/pkg/tbtcpg/chain.go @@ -148,13 +148,6 @@ type Chain interface { err error, ) - // GetMovedFundsSweepRequest gets the on-chain moved funds sweep request for - // the given moving funds transaction hash and output index. - GetMovedFundsSweepRequest( - movingFundsTxHash bitcoin.Hash, - movingFundsTxOutpointIndex uint32, - ) (*tbtc.MovedFundsSweepRequest, error) - // PastMovingFundsCommitmentSubmittedEvents fetches past moving funds // commitment submitted events according to the provided filter or // unfiltered if the filter is nil. Returned events are sorted by the block diff --git a/pkg/tbtcpg/chain_test.go b/pkg/tbtcpg/chain_test.go index 41d1e2abd1..5ddae422df 100644 --- a/pkg/tbtcpg/chain_test.go +++ b/pkg/tbtcpg/chain_test.go @@ -770,7 +770,7 @@ func (lc *LocalChain) GetMovingFundsParameters() ( func (lc *LocalChain) GetMovedFundsSweepRequest( movingFundsTxHash bitcoin.Hash, movingFundsTxOutpointIndex uint32, -) (*tbtc.MovedFundsSweepRequest, error) { +) (*tbtc.MovedFundsSweepRequest, bool, error) { lc.mutex.Lock() defer lc.mutex.Unlock() @@ -781,10 +781,10 @@ func (lc *LocalChain) GetMovedFundsSweepRequest( request, ok := lc.movedFundsSweepRequests[requestKey] if !ok { - return nil, fmt.Errorf("request not found") + return nil, false, nil } - return request, nil + return request, true, nil } func (lc *LocalChain) SetMovedFundsSweepRequest( diff --git a/pkg/tbtcpg/moved_funds_sweep.go b/pkg/tbtcpg/moved_funds_sweep.go index af46111b42..dfff664d77 100644 --- a/pkg/tbtcpg/moved_funds_sweep.go +++ b/pkg/tbtcpg/moved_funds_sweep.go @@ -174,7 +174,7 @@ func (mfst *MovedFundsSweepTask) FindMovingFundsTxData( // Find the first outpoint that represents an unproven moved funds sweep // request and return it. for _, outpoint := range movingFundsTxOutpoints { - movedFundsSweepRequest, err := mfst.chain.GetMovedFundsSweepRequest( + movedFundsSweepRequest, isRequest, err := mfst.chain.GetMovedFundsSweepRequest( outpoint.TransactionHash, outpoint.OutputIndex, ) @@ -185,6 +185,12 @@ func (mfst *MovedFundsSweepTask) FindMovingFundsTxData( ) } + // Check just in case. It should not happen, as all the outpoint should + // represent a valid request. + if !isRequest { + continue + } + if movedFundsSweepRequest.State == tbtc.MovedFundsStatePending { return outpoint.TransactionHash, outpoint.OutputIndex, nil } From c5768c558ccac5d71684a8be05eff7abf489e26b Mon Sep 17 00:00:00 2001 From: Tomasz Slabon Date: Fri, 15 Mar 2024 15:04:12 +0100 Subject: [PATCH 2/3] Fixed code comments --- pkg/maintainer/spv/moved_funds_sweep.go | 2 +- pkg/tbtc/wallet.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/maintainer/spv/moved_funds_sweep.go b/pkg/maintainer/spv/moved_funds_sweep.go index e45904890a..34f15424e0 100644 --- a/pkg/maintainer/spv/moved_funds_sweep.go +++ b/pkg/maintainer/spv/moved_funds_sweep.go @@ -275,7 +275,7 @@ func isUnprovenMovedFundsSweepTransaction( ) } - // Check is it's a moved funds sweep request at all. + // Check if it's a moved funds sweep request at all. if !isRequest { return false, nil } diff --git a/pkg/tbtc/wallet.go b/pkg/tbtc/wallet.go index f2c615808a..461c9ba3a5 100644 --- a/pkg/tbtc/wallet.go +++ b/pkg/tbtc/wallet.go @@ -653,7 +653,7 @@ func EnsureWalletSyncedBetweenChains( if isRequest { // If that's the case, the wallet has already created a moved - // funds sweep their first Bitcoin transaction and the Bridge + // funds sweep as their first Bitcoin transaction and the Bridge // is awaiting the SPV proof. return fmt.Errorf("wallet already produced their first " + "Bitcoin transaction (moved funds sweep); Bridge is " + From 13d5927dd351e5f78e408828feafa96ff8d26731 Mon Sep 17 00:00:00 2001 From: Tomasz Slabon Date: Fri, 15 Mar 2024 15:59:34 +0100 Subject: [PATCH 3/3] Fixed unit tests --- pkg/tbtc/chain_test.go | 33 ++++++++++++++++++++++++++++++++- 1 file changed, 32 insertions(+), 1 deletion(-) diff --git a/pkg/tbtc/chain_test.go b/pkg/tbtc/chain_test.go index 2f1ad5c9fc..be758757cc 100644 --- a/pkg/tbtc/chain_test.go +++ b/pkg/tbtc/chain_test.go @@ -69,6 +69,9 @@ type localChain struct { movingFundsProposalValidationsMutex sync.Mutex movingFundsProposalValidations map[[32]byte]bool + movedFundsSweepRequestsMutex sync.Mutex + movedFundsSweepRequests map[[32]byte]*MovedFundsSweepRequest + movedFundsSweepProposalValidationsMutex sync.Mutex movedFundsSweepProposalValidations map[[32]byte]bool @@ -202,11 +205,39 @@ func (lc *localChain) IsBetaOperator() (bool, error) { panic("unsupported") } +func buildMovedFundsSweepRequestKey( + movingFundsTxHash bitcoin.Hash, + movingFundsTxOutpointIndex uint32, +) [32]byte { + var buffer bytes.Buffer + + buffer.Write(movingFundsTxHash[:]) + + outputIndex := make([]byte, 4) + binary.BigEndian.PutUint32(outputIndex, movingFundsTxOutpointIndex) + buffer.Write(outputIndex) + + return sha256.Sum256(buffer.Bytes()) +} + func (lc *localChain) GetMovedFundsSweepRequest( movingFundsTxHash bitcoin.Hash, movingFundsTxOutpointIndex uint32, ) (*MovedFundsSweepRequest, bool, error) { - panic("unsupported") + lc.movedFundsSweepRequestsMutex.Lock() + defer lc.movedFundsSweepRequestsMutex.Unlock() + + requestKey := buildMovedFundsSweepRequestKey( + movingFundsTxHash, + movingFundsTxOutpointIndex, + ) + + request, ok := lc.movedFundsSweepRequests[requestKey] + if !ok { + return nil, false, nil + } + + return request, true, nil } func (lc *localChain) GetOperatorID(