Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Double spending tx are considered replacement #12301

Merged
merged 2 commits into from
Mar 13, 2024
Merged

Double spending tx are considered replacement #12301

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
94aece6
Double spending tx are considered replacement
lontivero Jan 23, 2024
3b57417
Update comment in code
lontivero Feb 5, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 5 additions & 5 deletions WalletWasabi.Tests/UnitTests/Transactions/TransactionProcessorTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -247,18 +247,18 @@ public async Task IgnoreDoubleSpendAsync()
var keys = transactionProcessor.KeyManager.GetKeys().ToArray();

// An unconfirmed segwit transaction for us
var tx0 = CreateCreditingTransaction(keys[0].PubKey.GetScriptPubKey(ScriptPubKeyType.Segwit), Money.Coins(1.0m));
var tx0 = CreateCreditingTransaction(keys[0].GetAssumedScriptPubKey(), Money.Coins(1.0m));

var createdCoin = tx0.Transaction.Outputs.AsCoins().First();

// Spend the received coin
var tx1 = CreateSpendingTransaction(createdCoin, keys[1].PubKey.GetScriptPubKey(ScriptPubKeyType.Segwit));
var tx1 = CreateSpendingTransaction(createdCoin, keys[1].GetAssumedScriptPubKey());

// Spend the same coin again
var tx2 = CreateSpendingTransaction(createdCoin, keys[2].PubKey.GetScriptPubKey(ScriptPubKeyType.Segwit));
var tx2 = CreateSpendingTransaction(createdCoin, keys[2].GetAssumedScriptPubKey());
var relevant = transactionProcessor.Process(tx0, tx1, tx2).Last();

Assert.False(relevant.IsNews);
Assert.True(relevant.IsNews);
Assert.Single(transactionProcessor.Coins, coin => !coin.IsSpent());
Assert.Single(transactionProcessor.Coins.AsAllCoinsView(), coin => coin.IsSpent());

Expand All @@ -267,7 +267,7 @@ public async Task IgnoreDoubleSpendAsync()
var mempool = transactionProcessor.TransactionStore.MempoolStore.GetTransactions();
Assert.Equal(2, mempool.Count);
Assert.Equal(tx0, mempool.First());
Assert.Equal(tx1, mempool.Last());
Assert.Equal(tx2, mempool.Last());
}

[Fact]
Expand Down
8 changes: 2 additions & 6 deletions WalletWasabi/Blockchain/TransactionProcessing/TransactionProcessor.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using NBitcoin;

Check notice on line 1 in WalletWasabi/Blockchain/TransactionProcessing/TransactionProcessor.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (master)

✅ Getting better: Overall Code Complexity 

The mean cyclomatic complexity decreases from 5.78 to 5.67, threshold = 4. This file has many conditional statements (e.g. if, for, while) across its implementation, leading to lower code health. Avoid adding more conditionals.
using System.Collections.Generic;
using System.Linq;
using WalletWasabi.Blockchain.Analysis;
Expand Down Expand Up @@ -146,24 +146,20 @@
if (tx.Height == Height.Mempool)
{
// if the received transaction is spending at least one input already
// spent by a previous unconfirmed transaction signaling RBF then it is not a double
// spent by a previous unconfirmed transaction then it is not considered a double
// spending transaction but a replacement transaction.
lontivero marked this conversation as resolved.
Show resolved Hide resolved
var isReplacementTx = doubleSpentSpenders.Any(x => x.Transaction.IsRBF);
var isReplacementTx = doubleSpentSpenders.Any();
if (isReplacementTx)
{
// Undo the replaced transaction by removing the coins it created (if other coin
// spends it, remove that too and so on) and restoring those that it replaced.
// After undoing the replaced transaction it will process the replacement transaction.
var replacedTxId = doubleSpentSpenders.First().TransactionId;
var (replaced, restored) = Coins.Undo(replacedTxId);

result.ReplacedCoins.AddRange(replaced);
result.RestoredCoins.AddRange(restored);
}

Check notice on line 162 in WalletWasabi/Blockchain/TransactionProcessing/TransactionProcessor.cs

View check run for this annotation

CodeScene Delta Analysis / CodeScene Cloud Delta Analysis (master)

✅ Getting better: Complex Method 

ProcessNoLock decreases in cyclomatic complexity from 34 to 33, threshold = 9. This function has many conditional statements (e.g. if, for, while), leading to lower code health. Avoid adding more conditionals and code to it without refactoring.
else if (doubleSpentSpenders.Count > 0)
{
return result;
}
}
else // new confirmation always enjoys priority
{
Expand Down