From ec00a8912fa7acfd2d90f9bb464f242efbf4ae25 Mon Sep 17 00:00:00 2001 From: Tomasz Slabon Date: Tue, 30 Apr 2024 19:15:55 +0200 Subject: [PATCH 1/7] Adjusted safety margin calculations when moving funds --- pkg/tbtc/chain.go | 26 +++++ pkg/tbtc/chain_test.go | 23 +++++ pkg/tbtc/moving_funds.go | 207 +++++++++++++++++++++++++++++++++++-- pkg/tbtcpg/chain.go | 17 --- pkg/tbtcpg/moving_funds.go | 2 +- 5 files changed, 247 insertions(+), 28 deletions(-) diff --git a/pkg/tbtc/chain.go b/pkg/tbtc/chain.go index 0a52004b60..7563b76b11 100644 --- a/pkg/tbtc/chain.go +++ b/pkg/tbtc/chain.go @@ -223,6 +223,32 @@ type BridgeChain interface { movingFundsTxHash bitcoin.Hash, movingFundsTxOutpointIndex uint32, ) (*MovedFundsSweepRequest, bool, error) + + // GetMovingFundsParameters gets the current value of parameters relevant + // for the moving funds process. + GetMovingFundsParameters() ( + txMaxTotalFee uint64, + dustThreshold uint64, + timeoutResetDelay uint32, + timeout uint32, + timeoutSlashingAmount *big.Int, + timeoutNotifierRewardMultiplier uint32, + commitmentGasOffset uint16, + sweepTxMaxTotalFee uint64, + sweepTimeout uint32, + sweepTimeoutSlashingAmount *big.Int, + sweepTimeoutNotifierRewardMultiplier uint32, + err 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 + // number in the ascending order, i.e. the latest event is at the end of the + // slice. + PastMovingFundsCommitmentSubmittedEvents( + filter *MovingFundsCommitmentSubmittedEventFilter, + ) ([]*MovingFundsCommitmentSubmittedEvent, error) } // NewWalletRegisteredEvent represents a new wallet registered event. diff --git a/pkg/tbtc/chain_test.go b/pkg/tbtc/chain_test.go index be758757cc..6e93b92a37 100644 --- a/pkg/tbtc/chain_test.go +++ b/pkg/tbtc/chain_test.go @@ -1099,6 +1099,29 @@ func buildMovedFundsSweepProposalValidationKey( return sha256.Sum256(buffer.Bytes()), nil } +func (lc *localChain) GetMovingFundsParameters() ( + txMaxTotalFee uint64, + dustThreshold uint64, + timeoutResetDelay uint32, + timeout uint32, + timeoutSlashingAmount *big.Int, + timeoutNotifierRewardMultiplier uint32, + commitmentGasOffset uint16, + sweepTxMaxTotalFee uint64, + sweepTimeout uint32, + sweepTimeoutSlashingAmount *big.Int, + sweepTimeoutNotifierRewardMultiplier uint32, + err error, +) { + panic("unsupported") +} + +func (lc *localChain) PastMovingFundsCommitmentSubmittedEvents( + filter *MovingFundsCommitmentSubmittedEventFilter, +) ([]*MovingFundsCommitmentSubmittedEvent, error) { + panic("unsupported") +} + // Connect sets up the local chain. func Connect(blockTime ...time.Duration) *localChain { operatorPrivateKey, _, err := operator.GenerateKeyPair(local_v1.DefaultCurve) diff --git a/pkg/tbtc/moving_funds.go b/pkg/tbtc/moving_funds.go index a030399414..b1d4b61639 100644 --- a/pkg/tbtc/moving_funds.go +++ b/pkg/tbtc/moving_funds.go @@ -9,6 +9,7 @@ import ( "github.com/keep-network/keep-common/pkg/chain/ethereum" "github.com/keep-network/keep-core/pkg/bitcoin" + "github.com/keep-network/keep-core/pkg/chain" "go.uber.org/zap" ) @@ -53,6 +54,11 @@ const ( movingFundsCommitmentConfirmationBlocks = 32 ) +// MovingFundsCommitmentLookBackBlocks is the look-back period in blocks used +// when searching for submitted moving funds commitment events. It's equal to +// 30 days assuming 12 seconds per block. +const MovingFundsCommitmentLookBackBlocks = uint64(216000) + // MovingFundsProposal represents a moving funds proposal issued by a wallet's // coordination leader. type MovingFundsProposal struct { @@ -312,20 +318,33 @@ func ValidateMovingFundsProposal( proposal *MovingFundsProposal, ) error + BlockCounter() (chain.BlockCounter, error) + GetWallet(walletPublicKeyHash [20]byte) (*WalletChainData, error) + + GetMovingFundsParameters() ( + txMaxTotalFee uint64, + dustThreshold uint64, + timeoutResetDelay uint32, + timeout uint32, + timeoutSlashingAmount *big.Int, + timeoutNotifierRewardMultiplier uint32, + commitmentGasOffset uint16, + sweepTxMaxTotalFee uint64, + sweepTimeout uint32, + sweepTimeoutSlashingAmount *big.Int, + sweepTimeoutNotifierRewardMultiplier uint32, + err error, + ) + + PastMovingFundsCommitmentSubmittedEvents( + filter *MovingFundsCommitmentSubmittedEventFilter, + ) ([]*MovingFundsCommitmentSubmittedEvent, error) }, ) error { validateProposalLogger.Infof("calling chain for proposal validation") - walletChainData, err := chain.GetWallet(walletPublicKeyHash) - if err != nil { - return fmt.Errorf( - "cannot get wallet's chain data: [%w]", - err, - ) - } - - err = ValidateMovingFundsSafetyMargin(walletChainData) + err := ValidateMovingFundsSafetyMargin(walletPublicKeyHash, chain) if err != nil { return fmt.Errorf("moving funds proposal is invalid: [%v]", err) } @@ -354,10 +373,87 @@ func ValidateMovingFundsProposal( // deposits so, it makes sense to preserve a safety margin before moving // funds to give the last minute deposits a chance to become eligible for // deposit sweep. +// +// Similarly, wallets that just entered the MovingFunds state may have become +// target wallets for another moving funds wallets. It makes sense to preserve +// a safety margin to allow the wallet to merge the moved funds from another +// wallets. In this case a longer safety margin should be used. func ValidateMovingFundsSafetyMargin( - walletChainData *WalletChainData, + walletPublicKeyHash [20]byte, + chain interface { + BlockCounter() (chain.BlockCounter, error) + + GetWallet(walletPublicKeyHash [20]byte) (*WalletChainData, error) + + GetMovingFundsParameters() ( + txMaxTotalFee uint64, + dustThreshold uint64, + timeoutResetDelay uint32, + timeout uint32, + timeoutSlashingAmount *big.Int, + timeoutNotifierRewardMultiplier uint32, + commitmentGasOffset uint16, + sweepTxMaxTotalFee uint64, + sweepTimeout uint32, + sweepTimeoutSlashingAmount *big.Int, + sweepTimeoutNotifierRewardMultiplier uint32, + err error, + ) + + PastMovingFundsCommitmentSubmittedEvents( + filter *MovingFundsCommitmentSubmittedEventFilter, + ) ([]*MovingFundsCommitmentSubmittedEvent, error) + }, ) error { + // In most cases the safety margin of 24 hours should be enough. It will + // allow the wallet to sweep the last deposits that were made before the + // wallet entered the moving funds state. safetyMargin := time.Duration(24) * time.Hour + + // It is possible that our wallet is the target wallet in another pending + // moving funds procedure. If this is the case we must apply a longer + // 14-day safety margin. This will ensure the funds moved from another + // wallet can be merged with our wallet's main UTXO before moving funds. + isMovingFundsTarget, err := isWalletPendingMovingFundsTarget( + walletPublicKeyHash, + chain, + ) + if err != nil { + return fmt.Errorf( + "cannot check if wallet is pending moving funds target: [%w]", + err, + ) + } + + if isMovingFundsTarget { + safetyMargin = time.Duration(24) * 14 * time.Hour + } + + // As the moving funds procedure is time constrained, we must ensure the + // safety margin does not exceed half of the moving funds timeout parameter. + // This should give the wallet enough time to complete moving funds. + _, _, _, movingFundsTimeout, _, _, _, _, _, _, _, err := + chain.GetMovingFundsParameters() + if err != nil { + return fmt.Errorf("cannot get moving funds parameters: [%w]", err) + } + + maxAllowedSafetyMargin := time.Duration( + float64(movingFundsTimeout) * 0.5 * float64(time.Second), + ) + + if safetyMargin > maxAllowedSafetyMargin { + safetyMargin = maxAllowedSafetyMargin + } + + walletChainData, err := chain.GetWallet(walletPublicKeyHash) + if err != nil { + return fmt.Errorf( + "cannot get wallet's chain data: [%w]", + err, + ) + } + safetyMarginExpiresAt := walletChainData.MovingFundsRequestedAt.Add(safetyMargin) if time.Now().Before(safetyMarginExpiresAt) { @@ -375,6 +471,97 @@ func (mfa *movingFundsAction) actionType() WalletActionType { return ActionMovingFunds } +func isWalletPendingMovingFundsTarget( + walletPublicKeyHash [20]byte, + + chain interface { + BlockCounter() (chain.BlockCounter, error) + + GetWallet(walletPublicKeyHash [20]byte) (*WalletChainData, error) + + GetMovingFundsParameters() ( + txMaxTotalFee uint64, + dustThreshold uint64, + timeoutResetDelay uint32, + timeout uint32, + timeoutSlashingAmount *big.Int, + timeoutNotifierRewardMultiplier uint32, + commitmentGasOffset uint16, + sweepTxMaxTotalFee uint64, + sweepTimeout uint32, + sweepTimeoutSlashingAmount *big.Int, + sweepTimeoutNotifierRewardMultiplier uint32, + err error, + ) + + PastMovingFundsCommitmentSubmittedEvents( + filter *MovingFundsCommitmentSubmittedEventFilter, + ) ([]*MovingFundsCommitmentSubmittedEvent, error) + }, +) (bool, error) { + blockCounter, err := chain.BlockCounter() + if err != nil { + return false, fmt.Errorf("failed to get block counter: [%w]", err) + } + + currentBlockNumber, err := blockCounter.CurrentBlock() + if err != nil { + return false, fmt.Errorf( + "failed to get current block number: [%w]", + err, + ) + } + + filterStartBlock := uint64(0) + if currentBlockNumber > MovingFundsCommitmentLookBackBlocks { + filterStartBlock = currentBlockNumber - MovingFundsCommitmentLookBackBlocks + } + + // Get all the recent moving funds commitment submitted events. + filter := &MovingFundsCommitmentSubmittedEventFilter{ + StartBlock: filterStartBlock, + } + + events, err := chain.PastMovingFundsCommitmentSubmittedEvents(filter) + if err != nil { + return false, fmt.Errorf( + "failed to get past moving funds commitment submitted events: [%w]", + err, + ) + } + + isWalletTarget := func(event *MovingFundsCommitmentSubmittedEvent) bool { + for _, targetWallet := range event.TargetWallets { + if walletPublicKeyHash == targetWallet { + return true + } + } + return false + } + + for _, event := range events { + if !isWalletTarget(event) { + continue + } + + // Our wallet is on the list of target wallets. If the state is moving + // funds, there is probably moving funds to our wallet in the process. + walletChainData, err := chain.GetWallet(walletPublicKeyHash) + if err != nil { + return false, fmt.Errorf( + "cannot get wallet's chain data: [%w]", + err, + ) + } + + if walletChainData.State == StateMovingFunds { + return true, nil + } + } + + return false, nil +} + func assembleMovingFundsTransaction( bitcoinChain bitcoin.Chain, walletMainUtxo *bitcoin.UnspentTransactionOutput, diff --git a/pkg/tbtcpg/chain.go b/pkg/tbtcpg/chain.go index 6a85f121ab..01e519c462 100644 --- a/pkg/tbtcpg/chain.go +++ b/pkg/tbtcpg/chain.go @@ -131,23 +131,6 @@ type Chain interface { proposal *tbtc.HeartbeatProposal, ) error - // GetMovingFundsParameters gets the current value of parameters relevant - // for the moving funds process. - GetMovingFundsParameters() ( - txMaxTotalFee uint64, - dustThreshold uint64, - timeoutResetDelay uint32, - timeout uint32, - timeoutSlashingAmount *big.Int, - timeoutNotifierRewardMultiplier uint32, - commitmentGasOffset uint16, - sweepTxMaxTotalFee uint64, - sweepTimeout uint32, - sweepTimeoutSlashingAmount *big.Int, - sweepTimeoutNotifierRewardMultiplier uint32, - err 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/moving_funds.go b/pkg/tbtcpg/moving_funds.go index 20cc8d7ba4..22a842abc8 100644 --- a/pkg/tbtcpg/moving_funds.go +++ b/pkg/tbtcpg/moving_funds.go @@ -95,7 +95,7 @@ func (mft *MovingFundsTask) Run(request *tbtc.CoordinationProposalRequest) ( // Check the safety margin for moving funds early. This will prevent // commitment submission if the wallet is not safe to move funds. - err = tbtc.ValidateMovingFundsSafetyMargin(walletChainData) + err = tbtc.ValidateMovingFundsSafetyMargin(walletPublicKeyHash, mft.chain) if err != nil { taskLogger.Infof("source wallet moving funds safety margin validation failed: [%v]", err) return nil, false, nil From c09573457ec17d82ae7063b01994d76a2a839fce Mon Sep 17 00:00:00 2001 From: Tomasz Slabon Date: Wed, 1 May 2024 17:55:35 +0200 Subject: [PATCH 2/7] Adjusted unit tests --- pkg/tbtc/chain_test.go | 152 ++++++++++++++++++++++++++++---- pkg/tbtc/moving_funds_test.go | 14 +++ pkg/tbtcpg/moving_funds_test.go | 17 +++- 3 files changed, 167 insertions(+), 16 deletions(-) diff --git a/pkg/tbtc/chain_test.go b/pkg/tbtc/chain_test.go index 6e93b92a37..a344b8b2b4 100644 --- a/pkg/tbtc/chain_test.go +++ b/pkg/tbtc/chain_test.go @@ -28,6 +28,20 @@ import ( const localChainOperatorID = chain.OperatorID(1) +type movingFundsParameters = struct { + txMaxTotalFee uint64 + dustThreshold uint64 + timeoutResetDelay uint32 + timeout uint32 + timeoutSlashingAmount *big.Int + timeoutNotifierRewardMultiplier uint32 + commitmentGasOffset uint16 + sweepTxMaxTotalFee uint64 + sweepTimeout uint32 + sweepTimeoutSlashingAmount *big.Int + sweepTimeoutNotifierRewardMultiplier uint32 +} + type localChain struct { dkgResultSubmissionHandlersMutex sync.Mutex dkgResultSubmissionHandlers map[int]func(submission *DKGResultSubmittedEvent) @@ -57,6 +71,9 @@ type localChain struct { pastDepositRevealedEventsMutex sync.Mutex pastDepositRevealedEvents map[[32]byte][]*DepositRevealedEvent + pastMovingFundsCommitmentSubmittedEventsMutex sync.Mutex + pastMovingFundsCommitmentSubmittedEvents map[[32]byte][]*MovingFundsCommitmentSubmittedEvent + depositSweepProposalValidationsMutex sync.Mutex depositSweepProposalValidations map[[32]byte]bool @@ -81,6 +98,9 @@ type localChain struct { depositRequestsMutex sync.Mutex depositRequests map[[32]byte]*DepositChainRequest + movingFundsParametersMutex sync.Mutex + movingFundsParameters movingFundsParameters + blockCounter chain.BlockCounter operatorPrivateKey *operator.PrivateKey } @@ -1113,13 +1133,114 @@ func (lc *localChain) GetMovingFundsParameters() ( sweepTimeoutNotifierRewardMultiplier uint32, err error, ) { - panic("unsupported") + lc.movingFundsParametersMutex.Lock() + defer lc.movingFundsParametersMutex.Unlock() + + return lc.movingFundsParameters.txMaxTotalFee, + lc.movingFundsParameters.dustThreshold, + lc.movingFundsParameters.timeoutResetDelay, + lc.movingFundsParameters.timeout, + lc.movingFundsParameters.timeoutSlashingAmount, + lc.movingFundsParameters.timeoutNotifierRewardMultiplier, + lc.movingFundsParameters.commitmentGasOffset, + lc.movingFundsParameters.sweepTxMaxTotalFee, + lc.movingFundsParameters.sweepTimeout, + lc.movingFundsParameters.sweepTimeoutSlashingAmount, + lc.movingFundsParameters.sweepTimeoutNotifierRewardMultiplier, + nil +} + +func (lc *localChain) SetMovingFundsParameters( + txMaxTotalFee uint64, + dustThreshold uint64, + timeoutResetDelay uint32, + timeout uint32, + timeoutSlashingAmount *big.Int, + timeoutNotifierRewardMultiplier uint32, + commitmentGasOffset uint16, + sweepTxMaxTotalFee uint64, + sweepTimeout uint32, + sweepTimeoutSlashingAmount *big.Int, + sweepTimeoutNotifierRewardMultiplier uint32, +) { + lc.movingFundsParametersMutex.Lock() + defer lc.movingFundsParametersMutex.Unlock() + + lc.movingFundsParameters = movingFundsParameters{ + txMaxTotalFee: txMaxTotalFee, + dustThreshold: dustThreshold, + timeoutResetDelay: timeoutResetDelay, + timeout: timeout, + timeoutSlashingAmount: timeoutSlashingAmount, + timeoutNotifierRewardMultiplier: timeoutNotifierRewardMultiplier, + commitmentGasOffset: commitmentGasOffset, + sweepTxMaxTotalFee: sweepTxMaxTotalFee, + sweepTimeout: sweepTimeout, + sweepTimeoutSlashingAmount: sweepTimeoutSlashingAmount, + sweepTimeoutNotifierRewardMultiplier: sweepTimeoutNotifierRewardMultiplier, + } } func (lc *localChain) PastMovingFundsCommitmentSubmittedEvents( filter *MovingFundsCommitmentSubmittedEventFilter, ) ([]*MovingFundsCommitmentSubmittedEvent, error) { - panic("unsupported") + lc.pastMovingFundsCommitmentSubmittedEventsMutex.Lock() + defer lc.pastMovingFundsCommitmentSubmittedEventsMutex.Unlock() + + eventsKey, err := buildPastMovingFundsCommitmentSubmittedEventsKey(filter) + if err != nil { + return nil, err + } + + events, ok := lc.pastMovingFundsCommitmentSubmittedEvents[eventsKey] + if !ok { + return nil, fmt.Errorf("no events for given filter") + } + + return events, nil +} + +func (lc *localChain) setPastMovingFundsCommitmentSubmittedEvents( + filter *MovingFundsCommitmentSubmittedEventFilter, + events []*MovingFundsCommitmentSubmittedEvent, +) error { + lc.pastMovingFundsCommitmentSubmittedEventsMutex.Lock() + defer lc.pastMovingFundsCommitmentSubmittedEventsMutex.Unlock() + + eventsKey, err := buildPastMovingFundsCommitmentSubmittedEventsKey(filter) + if err != nil { + return err + } + + lc.pastMovingFundsCommitmentSubmittedEvents[eventsKey] = events + + return nil +} + +func buildPastMovingFundsCommitmentSubmittedEventsKey( + filter *MovingFundsCommitmentSubmittedEventFilter, +) ([32]byte, error) { + if filter == nil { + return [32]byte{}, nil + } + + var buffer bytes.Buffer + + startBlock := make([]byte, 8) + binary.BigEndian.PutUint64(startBlock, filter.StartBlock) + buffer.Write(startBlock) + + if filter.EndBlock != nil { + endBlock := make([]byte, 8) + binary.BigEndian.PutUint64(startBlock, *filter.EndBlock) + buffer.Write(endBlock) + } + + for _, walletPublicKeyHash := range filter.WalletPublicKeyHash { + buffer.Write(walletPublicKeyHash[:]) + } + + return sha256.Sum256(buffer.Bytes()), nil } // Connect sets up the local chain. @@ -1150,19 +1271,20 @@ func ConnectWithKey( dkgResultChallengeHandlers: make( map[int]func(submission *DKGResultChallengedEvent), ), - wallets: make(map[[20]byte]*WalletChainData), - blocksByTimestamp: make(map[uint64]uint64), - blocksHashesByNumber: make(map[uint64][32]byte), - pastDepositRevealedEvents: make(map[[32]byte][]*DepositRevealedEvent), - depositSweepProposalValidations: make(map[[32]byte]bool), - pendingRedemptionRequests: make(map[[32]byte]*RedemptionRequest), - redemptionProposalValidations: make(map[[32]byte]bool), - movingFundsProposalValidations: make(map[[32]byte]bool), - movedFundsSweepProposalValidations: make(map[[32]byte]bool), - heartbeatProposalValidations: make(map[[16]byte]bool), - depositRequests: make(map[[32]byte]*DepositChainRequest), - blockCounter: blockCounter, - operatorPrivateKey: operatorPrivateKey, + wallets: make(map[[20]byte]*WalletChainData), + blocksByTimestamp: make(map[uint64]uint64), + blocksHashesByNumber: make(map[uint64][32]byte), + pastDepositRevealedEvents: make(map[[32]byte][]*DepositRevealedEvent), + pastMovingFundsCommitmentSubmittedEvents: make(map[[32]byte][]*MovingFundsCommitmentSubmittedEvent), + depositSweepProposalValidations: make(map[[32]byte]bool), + pendingRedemptionRequests: make(map[[32]byte]*RedemptionRequest), + redemptionProposalValidations: make(map[[32]byte]bool), + movingFundsProposalValidations: make(map[[32]byte]bool), + movedFundsSweepProposalValidations: make(map[[32]byte]bool), + heartbeatProposalValidations: make(map[[16]byte]bool), + depositRequests: make(map[[32]byte]*DepositChainRequest), + blockCounter: blockCounter, + operatorPrivateKey: operatorPrivateKey, } return localChain diff --git a/pkg/tbtc/moving_funds_test.go b/pkg/tbtc/moving_funds_test.go index 9e62fed20f..1570f8de02 100644 --- a/pkg/tbtc/moving_funds_test.go +++ b/pkg/tbtc/moving_funds_test.go @@ -48,6 +48,11 @@ func TestMovingFundsAction_Execute(t *testing.T) { proposalExpiryBlock := proposalProcessingStartBlock + movingFundsProposalValidityBlocks + // Set arbitrary moving funds timeout. + hostChain.SetMovingFundsParameters( + 0, 0, 0, 604800, big.NewInt(0), 0, 0, 0, 0, big.NewInt(0), 0, + ) + // Simulate the on-chain proposal validation passes with success. err = hostChain.setMovingFundsProposalValidationResult( walletPublicKeyHash, @@ -59,6 +64,15 @@ func TestMovingFundsAction_Execute(t *testing.T) { t.Fatal(err) } + // Simulate the wallet was not chosen as a target wallet for another + // moving funds wallet. + hostChain.setPastMovingFundsCommitmentSubmittedEvents( + &MovingFundsCommitmentSubmittedEventFilter{ + StartBlock: 0, + }, + []*MovingFundsCommitmentSubmittedEvent{}, + ) + // Record the wallet main UTXO hash and moving funds commitment // hash in the local host chain so the moving funds action can detect it. walletMainUtxoHash := hostChain.ComputeMainUtxoHash( diff --git a/pkg/tbtcpg/moving_funds_test.go b/pkg/tbtcpg/moving_funds_test.go index 469ad24da4..4f24ae8ef7 100644 --- a/pkg/tbtcpg/moving_funds_test.go +++ b/pkg/tbtcpg/moving_funds_test.go @@ -579,6 +579,12 @@ func TestMovingFundsAction_ProposeMovingFunds(t *testing.T) { tbtcChain := tbtcpg.NewLocalChain() btcChain := tbtcpg.NewLocalBitcoinChain() + currentBlock := uint64(200000) + + blockCounter := tbtcpg.NewMockBlockCounter() + blockCounter.SetCurrentBlock(currentBlock) + tbtcChain.SetBlockCounter(blockCounter) + btcChain.SetEstimateSatPerVByteFee(1, 25) tbtcChain.SetWallet( @@ -588,11 +594,20 @@ func TestMovingFundsAction_ProposeMovingFunds(t *testing.T) { }, ) + // Simulate the wallet was not chosen as a target wallet for another + // moving funds wallet. + tbtcChain.AddPastMovingFundsCommitmentSubmittedEvent( + &tbtc.MovingFundsCommitmentSubmittedEventFilter{ + StartBlock: 0, + }, + &tbtc.MovingFundsCommitmentSubmittedEvent{}, + ) + tbtcChain.SetMovingFundsParameters( txMaxTotalFee, 0, 0, - 0, + 604800, nil, 0, 0, From ed0d2464227f33eb97c6a3286174c6bed5aff80d Mon Sep 17 00:00:00 2001 From: Tomasz Slabon Date: Wed, 1 May 2024 19:54:16 +0200 Subject: [PATCH 3/7] Fixed error --- pkg/tbtc/moving_funds.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/tbtc/moving_funds.go b/pkg/tbtc/moving_funds.go index b1d4b61639..af844ec4a8 100644 --- a/pkg/tbtc/moving_funds.go +++ b/pkg/tbtc/moving_funds.go @@ -546,7 +546,7 @@ func isWalletPendingMovingFundsTarget( // Our wallet is on the list of target wallets. If the state is moving // funds, there is probably moving funds to our wallet in the process. - walletChainData, err := chain.GetWallet(walletPublicKeyHash) + walletChainData, err := chain.GetWallet(event.WalletPublicKeyHash) if err != nil { return false, fmt.Errorf( "cannot get wallet's chain data: [%w]", From 4264a5e35e9636b941c34a1f0ceb27fa4c567e10 Mon Sep 17 00:00:00 2001 From: Tomasz Slabon Date: Wed, 1 May 2024 20:23:52 +0200 Subject: [PATCH 4/7] Added unit tests for safety margin validation --- pkg/tbtc/moving_funds_test.go | 210 ++++++++++++++++++++++++++++++++++ 1 file changed, 210 insertions(+) diff --git a/pkg/tbtc/moving_funds_test.go b/pkg/tbtc/moving_funds_test.go index 1570f8de02..d1fb2b99d4 100644 --- a/pkg/tbtc/moving_funds_test.go +++ b/pkg/tbtc/moving_funds_test.go @@ -2,7 +2,10 @@ package tbtc import ( "context" + "encoding/hex" + "fmt" "math/big" + "reflect" "testing" "time" @@ -223,3 +226,210 @@ func TestAssembleMovingFundsTransaction(t *testing.T) { }) } } + +func TestValidateMovingFundsSafetyMargin(t *testing.T) { + walletPublicKeyHash := hexToByte20( + "ffb3f7538bfa98a511495dd96027cfbd57baf2fa", + ) + + var tests = map[string]struct { + events []*MovingFundsCommitmentSubmittedEvent + requestedAtDelta time.Duration + otherWallets []struct { + publicKeyHash [20]byte + state WalletState + } + movingFundsTimeout uint32 + expectedError error + }{ + "wallet is not target and safety margin passed": { + events: []*MovingFundsCommitmentSubmittedEvent{}, + requestedAtDelta: -25 * time.Hour, + movingFundsTimeout: 3628800, // 6 weeks + expectedError: nil, + }, + "wallet is not target and safety margin not passed": { + events: []*MovingFundsCommitmentSubmittedEvent{}, + requestedAtDelta: -23 * time.Hour, + movingFundsTimeout: 3628800, // 6 weeks + expectedError: fmt.Errorf("safety margin in force"), + }, + "wallet is target and safety margin passed": { + events: []*MovingFundsCommitmentSubmittedEvent{ + { + WalletPublicKeyHash: hexToByte20( + "3091d288521caec06ea912eacfd733edc5a36d6e", + ), + TargetWallets: [][20]byte{ + hexToByte20( + "c7302d75072d78be94eb8d36c4b77583c7abb06e", + ), + // wallet on the target wallets list + walletPublicKeyHash, + }, + }, + }, + requestedAtDelta: -15 * 24 * time.Hour, + otherWallets: []struct { + publicKeyHash [20]byte + state WalletState + }{ + { + publicKeyHash: hexToByte20("3091d288521caec06ea912eacfd733edc5a36d6e"), + state: StateMovingFunds, + }, + }, + movingFundsTimeout: 3628800, // 6 weeks + expectedError: nil, + }, + "wallet is target and safety margin not passed": { + events: []*MovingFundsCommitmentSubmittedEvent{ + { + WalletPublicKeyHash: hexToByte20( + "3091d288521caec06ea912eacfd733edc5a36d6e", + ), + TargetWallets: [][20]byte{ + hexToByte20( + "c7302d75072d78be94eb8d36c4b77583c7abb06e", + ), + // wallet on the target wallets list + walletPublicKeyHash, + }, + }, + }, + requestedAtDelta: -13 * 24 * time.Hour, + otherWallets: []struct { + publicKeyHash [20]byte + state WalletState + }{ + { + publicKeyHash: hexToByte20("3091d288521caec06ea912eacfd733edc5a36d6e"), + state: StateMovingFunds, + }, + }, + movingFundsTimeout: 3628800, // 6 weeks + expectedError: fmt.Errorf("safety margin in force"), + }, + "wallet is target and safety margin limited and passed": { + events: []*MovingFundsCommitmentSubmittedEvent{ + { + WalletPublicKeyHash: hexToByte20( + "3091d288521caec06ea912eacfd733edc5a36d6e", + ), + TargetWallets: [][20]byte{ + hexToByte20( + "c7302d75072d78be94eb8d36c4b77583c7abb06e", + ), + // wallet on the target wallets list + walletPublicKeyHash, + }, + }, + }, + requestedAtDelta: -8 * 24 * time.Hour, + otherWallets: []struct { + publicKeyHash [20]byte + state WalletState + }{ + { + publicKeyHash: hexToByte20("3091d288521caec06ea912eacfd733edc5a36d6e"), + state: StateMovingFunds, + }, + }, + // 2 weeks - it limits safety margin to 1 week (2 * 0.5) + movingFundsTimeout: 1209600, + expectedError: nil, + }, + "wallet is target and safety margin limited and not passed": { + events: []*MovingFundsCommitmentSubmittedEvent{ + { + WalletPublicKeyHash: hexToByte20( + "3091d288521caec06ea912eacfd733edc5a36d6e", + ), + TargetWallets: [][20]byte{ + hexToByte20( + "c7302d75072d78be94eb8d36c4b77583c7abb06e", + ), + // wallet on the target wallets list + walletPublicKeyHash, + }, + }, + }, + requestedAtDelta: -6 * 24 * time.Hour, + otherWallets: []struct { + publicKeyHash [20]byte + state WalletState + }{ + { + publicKeyHash: hexToByte20("3091d288521caec06ea912eacfd733edc5a36d6e"), + state: StateMovingFunds, + }, + }, + // 2 weeks - it limits safety margin to 1 week (2 * 0.5) + movingFundsTimeout: 1209600, + expectedError: fmt.Errorf("safety margin in force"), + }, + } + + for testName, test := range tests { + t.Run(testName, func(t *testing.T) { + hostChain := Connect() + + hostChain.SetMovingFundsParameters( + 0, + 0, + 0, + test.movingFundsTimeout, + big.NewInt(0), + 0, + 0, + 0, + 0, + big.NewInt(0), + 0, + ) + + hostChain.setPastMovingFundsCommitmentSubmittedEvents( + &MovingFundsCommitmentSubmittedEventFilter{ + StartBlock: 0, + }, + test.events, + ) + + hostChain.setWallet(walletPublicKeyHash, &WalletChainData{ + MovingFundsRequestedAt: time.Now().Add(test.requestedAtDelta), + }) + + for _, wallet := range test.otherWallets { + hostChain.setWallet(wallet.publicKeyHash, &WalletChainData{ + State: wallet.state, + }) + } + + err := ValidateMovingFundsSafetyMargin( + walletPublicKeyHash, + hostChain, + ) + + if !reflect.DeepEqual(test.expectedError, err) { + t.Errorf( + "unexpected error\nexpected: %v\nactual: %v\n", + test.expectedError, + err, + ) + } + }) + } +} + +func hexToByte20(hexStr string) [20]byte { + if len(hexStr) != 40 { + panic("hex string length incorrect") + } + decoded, err := hex.DecodeString(hexStr) + if err != nil { + panic(err) + } + var result [20]byte + copy(result[:], decoded) + return result +} From 9ea08640e0835efb2694e18949ac3af90a10bf57 Mon Sep 17 00:00:00 2001 From: Tomasz Slabon Date: Mon, 6 May 2024 14:10:23 +0200 Subject: [PATCH 5/7] Simplified max allowed timeout calculation --- pkg/tbtc/moving_funds.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/pkg/tbtc/moving_funds.go b/pkg/tbtc/moving_funds.go index af844ec4a8..e259bb6c23 100644 --- a/pkg/tbtc/moving_funds.go +++ b/pkg/tbtc/moving_funds.go @@ -438,9 +438,7 @@ func ValidateMovingFundsSafetyMargin( return fmt.Errorf("cannot get moving funds parameters: [%w]", err) } - maxAllowedSafetyMargin := time.Duration( - float64(movingFundsTimeout) * 0.5 * float64(time.Second), - ) + maxAllowedSafetyMargin := time.Duration(movingFundsTimeout/2) * time.Second if safetyMargin > maxAllowedSafetyMargin { safetyMargin = maxAllowedSafetyMargin From 48b14304dced9d0f13ca00fae4f7e9a89082b92a Mon Sep 17 00:00:00 2001 From: Tomasz Slabon Date: Mon, 6 May 2024 14:12:36 +0200 Subject: [PATCH 6/7] Simplified duration calculation --- pkg/tbtc/moving_funds.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/tbtc/moving_funds.go b/pkg/tbtc/moving_funds.go index e259bb6c23..67f5fe563c 100644 --- a/pkg/tbtc/moving_funds.go +++ b/pkg/tbtc/moving_funds.go @@ -426,7 +426,7 @@ func ValidateMovingFundsSafetyMargin( } if isMovingFundsTarget { - safetyMargin = time.Duration(24) * 14 * time.Hour + safetyMargin = time.Duration(14*24) * time.Hour } // As the moving funds procedure is time constrained, we must ensure the From 506d878ef2841d95fad62a66c61334a98735f400 Mon Sep 17 00:00:00 2001 From: Tomasz Slabon Date: Mon, 6 May 2024 14:16:52 +0200 Subject: [PATCH 7/7] Corrected comment --- pkg/tbtc/moving_funds.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/tbtc/moving_funds.go b/pkg/tbtc/moving_funds.go index 67f5fe563c..1e9c01b0a3 100644 --- a/pkg/tbtc/moving_funds.go +++ b/pkg/tbtc/moving_funds.go @@ -471,7 +471,6 @@ func (mfa *movingFundsAction) actionType() WalletActionType { func isWalletPendingMovingFundsTarget( walletPublicKeyHash [20]byte, - chain interface { BlockCounter() (chain.BlockCounter, error) @@ -542,8 +541,9 @@ func isWalletPendingMovingFundsTarget( continue } - // Our wallet is on the list of target wallets. If the state is moving - // funds, there is probably moving funds to our wallet in the process. + // Our wallet is on the list of target wallets. If the state of the + // source wallet is still MovingFunds, the moving funds process + // targeting our wallet is likely in progress. walletChainData, err := chain.GetWallet(event.WalletPublicKeyHash) if err != nil { return false, fmt.Errorf(