Merge pull request #567 from rsksmart/feature/GBI-2221

Feature/GBI-2221 - Remove coinbase transaction information registration

internal/adapters/dataproviders/bitcoin/common.go

@@ -87,12 +87,12 @@ func buildMerkleBranch(merkleTree []*chainhash.Hash, txCount uint32, txIndex uin
 	}
 }
 
-func serializePartialMerkleTree(txHash *chainhash.Hash, block *btcutil.Block, witness bool) ([]byte, error) {
+func serializePartialMerkleTree(txHash *chainhash.Hash, block *btcutil.Block) ([]byte, error) {
 	var err error
 	filter := bloom.NewFilter(1, 0, 0, wire.BloomUpdateAll)
 	filter.AddHash(txHash)
 
-	msg, indices := NewMerkleBlock(block, filter, witness)
+	msg, indices := bloom.NewMerkleBlock(block, filter)
 	if len(indices) > 1 {
 		return nil, fmt.Errorf("block matches more than one transaction (%v)", len(indices))
 	}

internal/adapters/dataproviders/bitcoin/rpc.go

@@ -109,7 +109,7 @@ func (rpc *bitcoindRpc) GetRawTransaction(hash string) ([]byte, error) {
 	}
 
 	var buf bytes.Buffer
-	if err = rawTx.MsgTx().Serialize(&buf); err != nil {
+	if err = rawTx.MsgTx().SerializeNoWitness(&buf); err != nil {
 		return nil, err
 	}
 	return buf.Bytes(), nil
@@ -122,12 +122,7 @@ func (rpc *bitcoindRpc) GetPartialMerkleTree(hash string) ([]byte, error) {
 	}
 
 	block := btcutil.NewBlock(rawBlock)
-	for _, t := range block.Transactions() {
-		if t.Hash().String() == hash && t.HasWitness() {
-			return serializePartialMerkleTree(parsedTxHash, block, true)
-		}
-	}
-	return serializePartialMerkleTree(parsedTxHash, block, false)
+	return serializePartialMerkleTree(parsedTxHash, block)
 }
 
 func (rpc *bitcoindRpc) GetHeight() (*big.Int, error) {
@@ -139,8 +134,6 @@ func (rpc *bitcoindRpc) GetHeight() (*big.Int, error) {
 }
 
 func (rpc *bitcoindRpc) BuildMerkleBranch(txHash string) (blockchain.MerkleBranch, error) {
-	var wid *chainhash.Hash
-	isWitness := false
 	rawBlock, parsedTxHash, err := rpc.getTxBlock(txHash)
 	if err != nil {
 		return blockchain.MerkleBranch{}, err
@@ -149,24 +142,19 @@ func (rpc *bitcoindRpc) BuildMerkleBranch(txHash string) (blockchain.MerkleBranc
 	block := btcutil.NewBlock(rawBlock)
 	txs := make([]*btcutil.Tx, 0)
 	for _, t := range block.MsgBlock().Transactions {
-		parsedTx := btcutil.NewTx(t)
-		if parsedTx.Hash().String() == txHash && parsedTx.HasWitness() {
-			isWitness = true
-			wid = parsedTx.WitnessHash()
-		}
-		txs = append(txs, parsedTx)
+		txs = append(txs, btcutil.NewTx(t))
 	}
 
 	var cleanStore []*chainhash.Hash
-	store := merkle.BuildMerkleTreeStore(txs, isWitness)
+	store := merkle.BuildMerkleTreeStore(txs, false)
 	for _, node := range store {
 		if node != nil {
 			cleanStore = append(cleanStore, node)
 		}
 	}
 
 	index := slices.IndexFunc(cleanStore, func(h *chainhash.Hash) bool {
-		return h != nil && (h.IsEqual(parsedTxHash) || h.IsEqual(wid))
+		return h != nil && h.IsEqual(parsedTxHash)
 	})
 	if index == -1 {
 		return blockchain.MerkleBranch{}, fmt.Errorf("transaction %s not found in merkle tree", txHash)
@@ -226,7 +214,7 @@ func (rpc *bitcoindRpc) GetCoinbaseInformation(txHash string) (blockchain.BtcCoi
 		}
 		txs = append(txs, btcutil.NewTx(tx))
 	}
-	pmt, err := serializePartialMerkleTree(&coinbaseTxHash, btcutil.NewBlock(block), false)
+	pmt, err := serializePartialMerkleTree(&coinbaseTxHash, btcutil.NewBlock(block))
 	if err != nil {
 		return blockchain.BtcCoinbaseTransactionInformation{}, err
 	}

internal/usecases/common.go

@@ -173,6 +173,8 @@ func SignConfiguration[C liquidity_provider.ConfigurationType](
 	return signedConfig, nil
 }
 
+// RegisterCoinbaseTransaction registers the information of the coinbase transaction of the block of a specific transaction in the Rootstock Bridge.
+// IMPORTANT: this function should not be called right now for security reasons. It is in the codebase for future compatibility but should not be used for now.
 func RegisterCoinbaseTransaction(btcRpc blockchain.BitcoinNetwork, bridgeContract blockchain.RootstockBridge, tx blockchain.BitcoinTransactionInformation) error {
 	if !tx.HasWitness {
 		return nil

internal/usecases/pegin/register_pegin.go

@@ -39,7 +39,6 @@ func (useCase *RegisterPeginUseCase) Run(ctx context.Context, retainedQuote quot
 	var err error
 	var peginQuote *quote.PeginQuote
 	var params blockchain.RegisterPeginParams
-	var userBtcTx blockchain.BitcoinTransactionInformation
 
 	if retainedQuote.State != quote.PeginStateCallForUserSucceeded {
 		return useCase.publishErrorEvent(ctx, retainedQuote, usecases.WrongStateError, true)
@@ -51,7 +50,7 @@ func (useCase *RegisterPeginUseCase) Run(ctx context.Context, retainedQuote quot
 		return useCase.publishErrorEvent(ctx, retainedQuote, usecases.QuoteNotFoundError, false)
 	}
 
-	if userBtcTx, err = useCase.getUserBtcTransactionIfValid(ctx, retainedQuote); err != nil {
+	if err = useCase.validateTransaction(ctx, retainedQuote); err != nil {
 		return err
 	}
 
@@ -62,10 +61,6 @@ func (useCase *RegisterPeginUseCase) Run(ctx context.Context, retainedQuote quot
 	useCase.rskWalletMutex.Lock()
 	defer useCase.rskWalletMutex.Unlock()
 
-	if err = usecases.RegisterCoinbaseTransaction(useCase.rpc.Btc, useCase.contracts.Bridge, userBtcTx); err != nil {
-		return useCase.publishErrorEvent(ctx, retainedQuote, err, errors.Is(err, blockchain.WaitingForBridgeError))
-	}
-
 	return useCase.performRegisterPegin(ctx, params, retainedQuote)
 }
 
@@ -119,15 +114,15 @@ func (useCase *RegisterPeginUseCase) buildRegisterPeginParams(peginQuote quote.P
 	}, nil
 }
 
-func (useCase *RegisterPeginUseCase) getUserBtcTransactionIfValid(ctx context.Context, retainedQuote quote.RetainedPeginQuote) (blockchain.BitcoinTransactionInformation, error) {
+func (useCase *RegisterPeginUseCase) validateTransaction(ctx context.Context, retainedQuote quote.RetainedPeginQuote) error {
 	var txInfo blockchain.BitcoinTransactionInformation
 	var err error
 	if txInfo, err = useCase.rpc.Btc.GetTransactionInfo(retainedQuote.UserBtcTxHash); err != nil {
-		return blockchain.BitcoinTransactionInformation{}, useCase.publishErrorEvent(ctx, retainedQuote, err, true)
+		return useCase.publishErrorEvent(ctx, retainedQuote, err, true)
 	} else if txInfo.Confirmations < useCase.contracts.Bridge.GetRequiredTxConfirmations() {
-		return blockchain.BitcoinTransactionInformation{}, useCase.publishErrorEvent(ctx, retainedQuote, usecases.NoEnoughConfirmationsError, true)
+		return useCase.publishErrorEvent(ctx, retainedQuote, usecases.NoEnoughConfirmationsError, true)
 	}
-	return txInfo, nil
+	return nil
 }
 
 func (useCase *RegisterPeginUseCase) performRegisterPegin(ctx context.Context, params blockchain.RegisterPeginParams, retainedQuote quote.RetainedPeginQuote) error {

internal/usecases/pegin/register_pegin_test.go

@@ -2,13 +2,10 @@ package pegin_test
 
 import (
 	"context"
-	"encoding/hex"
-	"errors"
 	"fmt"
 	"github.com/rsksmart/liquidity-provider-server/internal/entities"
 	"github.com/rsksmart/liquidity-provider-server/internal/entities/blockchain"
 	"github.com/rsksmart/liquidity-provider-server/internal/entities/quote"
-	"github.com/rsksmart/liquidity-provider-server/internal/entities/utils"
 	"github.com/rsksmart/liquidity-provider-server/internal/usecases"
 	"github.com/rsksmart/liquidity-provider-server/internal/usecases/pegin"
 	"github.com/rsksmart/liquidity-provider-server/test"
@@ -92,6 +89,7 @@ func TestRegisterPeginUseCase_Run(t *testing.T) {
 	bridge.AssertExpectations(t)
 	btc.AssertExpectations(t)
 	mutex.AssertExpectations(t)
+	bridge.AssertNotCalled(t, "RegisterBtcCoinbaseTransaction")
 }
 
 func TestRegisterPeginUseCase_Run_DontPublishRecoverableErrors(t *testing.T) {
@@ -493,66 +491,3 @@ func registerPeginUpdateErrorSetups(t *testing.T, registerPeginTx string, retain
 		},
 	}
 }
-
-func TestRegisterPeginUseCase_Run_RegisterCoinbase(t *testing.T) {
-	retainedPeginQuote := quote.RetainedPeginQuote{
-		QuoteHash:      hex.EncodeToString(utils.MustGetRandomBytes(32)),
-		Signature:      hex.EncodeToString(utils.MustGetRandomBytes(32)),
-		DepositAddress: test.AnyAddress, RequiredLiquidity: entities.NewWei(1500),
-		State:         quote.PeginStateCallForUserSucceeded,
-		UserBtcTxHash: userBtcTx, CallForUserTxHash: cfuTx,
-	}
-	lbc := new(mocks.LbcMock)
-	quoteRepository := new(mocks.PeginQuoteRepositoryMock)
-	eventBus := new(mocks.EventBusMock)
-	bridge := new(mocks.BridgeMock)
-	btc := new(mocks.BtcRpcMock)
-	mutex := new(mocks.MutexMock)
-	coinbaseInfo := blockchain.BtcCoinbaseTransactionInformation{BlockHash: utils.To32Bytes(utils.MustGetRandomBytes(32))}
-	// Mocks that don't change per test
-	mutex.On("Lock").Return().Times(3)
-	mutex.On("Unlock").Return().Times(3)
-	quoteRepository.EXPECT().GetQuote(test.AnyCtx, mock.Anything).Return(&testPeginQuote, nil).Times(3)
-	quoteRepository.EXPECT().UpdateRetainedQuote(test.AnyCtx, mock.Anything).Return(nil).Twice()
-	btc.On("GetRawTransaction", retainedPeginQuote.UserBtcTxHash).Return(btcRawTxMock, nil).Times(3)
-	btc.On("GetPartialMerkleTree", retainedPeginQuote.UserBtcTxHash).Return(pmtMock, nil).Times(3)
-	btc.On("GetTransactionBlockInfo", retainedPeginQuote.UserBtcTxHash).Return(btcBlockInfoMock, nil).Times(3)
-	bridge.On("GetRequiredTxConfirmations").Return(uint64(10)).Times(3)
-	btc.On("GetTransactionInfo", retainedPeginQuote.UserBtcTxHash).Return(blockchain.BitcoinTransactionInformation{
-		Hash: retainedPeginQuote.UserBtcTxHash, Confirmations: 10, HasWitness: true,
-		Outputs: map[string][]*entities.Wei{retainedPeginQuote.DepositAddress: {entities.NewWei(1)}},
-	}, nil).Times(3)
-	btc.On("GetCoinbaseInformation", retainedPeginQuote.UserBtcTxHash).Return(coinbaseInfo, nil).Times(3)
-	// once as it'll be called only on 1st test
-	lbc.On("RegisterPegin", mock.Anything).Return(registerPeginTx, nil).Once()
-
-	useCase := pegin.NewRegisterPeginUseCase(blockchain.RskContracts{Lbc: lbc, Bridge: bridge}, quoteRepository, eventBus, blockchain.Rpc{Btc: btc}, mutex)
-	t.Run("Should call RegisterCoinbaseTransaction", func(t *testing.T) {
-		bridge.On("RegisterBtcCoinbaseTransaction", coinbaseInfo).Return(test.AnyHash, nil).Once()
-		eventBus.On("Publish", mock.MatchedBy(func(e quote.RegisterPeginCompletedEvent) bool {
-			return e.Error == nil
-		})).Return().Once()
-		err := useCase.Run(context.Background(), retainedPeginQuote)
-		require.NoError(t, err)
-	})
-	t.Run("Should return recoverable error if tx wasn't registered due to waiting for the bridge", func(t *testing.T) {
-		bridge.On("RegisterBtcCoinbaseTransaction", coinbaseInfo).Return("", blockchain.WaitingForBridgeError).Once()
-		err := useCase.Run(context.Background(), retainedPeginQuote)
-		require.Error(t, err)
-		require.NotErrorIs(t, err, usecases.NonRecoverableError)
-	})
-	t.Run("Should return non recoverable error if tx wasn't registered due to any other error", func(t *testing.T) {
-		bridge.On("RegisterBtcCoinbaseTransaction", coinbaseInfo).Return("", assert.AnError).Once()
-		eventBus.On("Publish", mock.MatchedBy(func(e quote.RegisterPeginCompletedEvent) bool {
-			return errors.Is(e.Error, usecases.NonRecoverableError)
-		})).Return().Once()
-		err := useCase.Run(context.Background(), retainedPeginQuote)
-		require.ErrorIs(t, err, usecases.NonRecoverableError)
-	})
-	mutex.AssertExpectations(t)
-	lbc.AssertExpectations(t)
-	bridge.AssertExpectations(t)
-	btc.AssertExpectations(t)
-	quoteRepository.AssertExpectations(t)
-	eventBus.AssertExpectations(t)
-}

internal/usecases/pegout/refund_pegout.go

@@ -43,7 +43,6 @@ func NewRefundPegoutUseCase(
 func (useCase *RefundPegoutUseCase) Run(ctx context.Context, retainedQuote quote.RetainedPegoutQuote) error {
 	var params blockchain.RefundPegoutParams
 	var pegoutQuote *quote.PegoutQuote
-	var lpBtcTransaction blockchain.BitcoinTransactionInformation
 	var err error
 
 	if retainedQuote.State != quote.PegoutStateSendPegoutSucceeded {
@@ -56,7 +55,7 @@ func (useCase *RefundPegoutUseCase) Run(ctx context.Context, retainedQuote quote
 		return useCase.publishErrorEvent(ctx, retainedQuote, usecases.QuoteNotFoundError, false)
 	}
 
-	if lpBtcTransaction, err = useCase.getLpBtcTransactionIfValid(ctx, *pegoutQuote, retainedQuote); err != nil {
+	if err = useCase.validateBtcTransaction(ctx, *pegoutQuote, retainedQuote); err != nil {
 		return err
 	}
 
@@ -68,10 +67,6 @@ func (useCase *RefundPegoutUseCase) Run(ctx context.Context, retainedQuote quote
 	useCase.rskWalletMutex.Lock()
 	defer useCase.rskWalletMutex.Unlock()
 
-	if err = usecases.RegisterCoinbaseTransaction(useCase.rpc.Btc, useCase.contracts.Bridge, lpBtcTransaction); err != nil {
-		return useCase.publishErrorEvent(ctx, retainedQuote, err, errors.Is(err, blockchain.WaitingForBridgeError))
-	}
-
 	if _, err = useCase.performRefundPegout(ctx, retainedQuote, txConfig, params); err != nil {
 		return err
 	}
@@ -164,17 +159,17 @@ func (useCase *RefundPegoutUseCase) performRefundPegout(
 	return retainedQuote, nil
 }
 
-func (useCase *RefundPegoutUseCase) getLpBtcTransactionIfValid(
+func (useCase *RefundPegoutUseCase) validateBtcTransaction(
 	ctx context.Context,
 	pegoutQuote quote.PegoutQuote,
 	retainedQuote quote.RetainedPegoutQuote,
-) (blockchain.BitcoinTransactionInformation, error) {
+) error {
 	var txInfo blockchain.BitcoinTransactionInformation
 	var err error
 	if txInfo, err = useCase.rpc.Btc.GetTransactionInfo(retainedQuote.LpBtcTxHash); err != nil {
-		return blockchain.BitcoinTransactionInformation{}, useCase.publishErrorEvent(ctx, retainedQuote, err, true)
+		return useCase.publishErrorEvent(ctx, retainedQuote, err, true)
 	} else if txInfo.Confirmations < uint64(pegoutQuote.TransferConfirmations) {
-		return blockchain.BitcoinTransactionInformation{}, useCase.publishErrorEvent(ctx, retainedQuote, usecases.NoEnoughConfirmationsError, true)
+		return useCase.publishErrorEvent(ctx, retainedQuote, usecases.NoEnoughConfirmationsError, true)
 	}
-	return txInfo, nil
+	return nil
 }

internal/usecases/pegout/refund_pegout_test.go

@@ -6,7 +6,6 @@ import (
 	"github.com/rsksmart/liquidity-provider-server/internal/entities"
 	"github.com/rsksmart/liquidity-provider-server/internal/entities/blockchain"
 	"github.com/rsksmart/liquidity-provider-server/internal/entities/quote"
-	"github.com/rsksmart/liquidity-provider-server/internal/entities/utils"
 	"github.com/rsksmart/liquidity-provider-server/internal/usecases"
 	"github.com/rsksmart/liquidity-provider-server/internal/usecases/pegout"
 	"github.com/rsksmart/liquidity-provider-server/test"
@@ -111,8 +110,9 @@ func TestRefundPegoutUseCase_Run(t *testing.T) {
 	mutex := new(mocks.MutexMock)
 	mutex.On("Lock").Return().Once()
 	mutex.On("Unlock").Return().Once()
+	bridge := new(mocks.BridgeMock)
 
-	contracts := blockchain.RskContracts{Lbc: lbc}
+	contracts := blockchain.RskContracts{Lbc: lbc, Bridge: bridge}
 	rpc := blockchain.Rpc{Btc: btc}
 	useCase := pegout.NewRefundPegoutUseCase(quoteRepository, contracts, eventBus, rpc, mutex)
 	err := useCase.Run(context.Background(), retainedQuote)
@@ -121,6 +121,7 @@ func TestRefundPegoutUseCase_Run(t *testing.T) {
 	eventBus.AssertExpectations(t)
 	btc.AssertExpectations(t)
 	require.NoError(t, err)
+	bridge.AssertNotCalled(t, "RegisterBtcCoinbaseTransaction")
 }
 
 func TestRefundPegoutUseCase_Run_UpdateError(t *testing.T) {
@@ -327,59 +328,3 @@ func TestRefundPegoutUseCase_Run_WrongState(t *testing.T) {
 	mutex.AssertNotCalled(t, "Unlock")
 	require.ErrorIs(t, err, usecases.WrongStateError)
 }
-
-func TestRefundPegoutUseCase_Run_RegisterCoinbase(t *testing.T) {
-	quoteRepository := new(mocks.PegoutQuoteRepositoryMock)
-	lbc := new(mocks.LbcMock)
-	bridge := new(mocks.BridgeMock)
-	eventBus := new(mocks.EventBusMock)
-	btc := new(mocks.BtcRpcMock)
-	mutex := new(mocks.MutexMock)
-	coinbaseInfo := blockchain.BtcCoinbaseTransactionInformation{BlockHash: utils.To32Bytes(utils.MustGetRandomBytes(32))}
-	// Mocks that don't change per test
-	mutex.On("Lock").Return().Times(3)
-	mutex.On("Unlock").Return().Times(3)
-	quoteRepository.EXPECT().UpdateRetainedQuote(test.AnyCtx, mock.Anything).Return(nil).Twice()
-	quoteRepository.EXPECT().GetQuote(test.AnyCtx, retainedQuote.QuoteHash).Return(&pegoutQuote, nil).Times(3)
-	tx := btcTxInfoMock
-	tx.HasWitness = true
-	btc.On("GetTransactionInfo", retainedQuote.LpBtcTxHash).Return(tx, nil).Times(3)
-	btc.On("GetCoinbaseInformation", retainedQuote.LpBtcTxHash).Return(coinbaseInfo, nil).Times(3)
-	btc.On("BuildMerkleBranch", mock.Anything).Return(merkleBranchMock, nil).Times(3)
-	btc.On("GetRawTransaction", mock.Anything).Return(btcRawTxMock, nil).Times(3)
-	btc.On("GetTransactionBlockInfo", mock.Anything).Return(btcBlockInfoMock, nil).Times(3)
-	// once as it'll be called only on 1st test
-	lbc.On("RefundPegout", mock.Anything, mock.Anything).Return(refundPegoutTxHash, nil).Once()
-
-	contracts := blockchain.RskContracts{Lbc: lbc, Bridge: bridge}
-	rpc := blockchain.Rpc{Btc: btc}
-	useCase := pegout.NewRefundPegoutUseCase(quoteRepository, contracts, eventBus, rpc, mutex)
-	t.Run("Should call RegisterCoinbaseTransaction", func(t *testing.T) {
-		bridge.On("RegisterBtcCoinbaseTransaction", coinbaseInfo).Return(test.AnyHash, nil).Once()
-		eventBus.On("Publish", mock.MatchedBy(func(e quote.PegoutQuoteCompletedEvent) bool {
-			return e.Error == nil
-		})).Return().Once()
-		err := useCase.Run(context.Background(), retainedQuote)
-		require.NoError(t, err)
-	})
-	t.Run("Should return recoverable error if tx wasn't registered due to waiting for the bridge", func(t *testing.T) {
-		bridge.On("RegisterBtcCoinbaseTransaction", coinbaseInfo).Return("", blockchain.WaitingForBridgeError).Once()
-		err := useCase.Run(context.Background(), retainedQuote)
-		require.Error(t, err)
-		require.NotErrorIs(t, err, usecases.NonRecoverableError)
-	})
-	t.Run("Should return non recoverable error if tx wasn't registered due to any other error", func(t *testing.T) {
-		bridge.On("RegisterBtcCoinbaseTransaction", coinbaseInfo).Return("", assert.AnError).Once()
-		eventBus.On("Publish", mock.MatchedBy(func(e quote.PegoutQuoteCompletedEvent) bool {
-			return errors.Is(e.Error, usecases.NonRecoverableError)
-		})).Return().Once()
-		err := useCase.Run(context.Background(), retainedQuote)
-		require.ErrorIs(t, err, usecases.NonRecoverableError)
-	})
-	mutex.AssertExpectations(t)
-	lbc.AssertExpectations(t)
-	bridge.AssertExpectations(t)
-	btc.AssertExpectations(t)
-	quoteRepository.AssertExpectations(t)
-	eventBus.AssertExpectations(t)
-}