Finalize stale pending multisig transactions on read
What the developer wrote
Authored by rockstardev
Finalize stale pending multisig transactions on read
What changed, and why it matters
This commit fixes a bookkeeping bug in BTCPay Server's multisig (multi-signature) transaction tracker. Previously, a pending transaction could silently collect enough signatures to be finalized, but the system would still label it as 'Pending' until something else happened. The change makes the service automatically mark such transactions as 'Signed' whenever they are read, and it adds tests to confirm the state is corrected. There is no direct evidence this was a security vulnerability or that it was exploited; it appears to be a state-consistency fix.
Treat as a normal bug-fix commit. Review whether the state transition could be triggered by a maliciously crafted PSBT and ensure TryFinalize failures are handled safely. No urgent security response is indicated by the supplied materials.
Security signals we found
State-machine inconsistency: persisted transaction state could diverge from actual signature count
Multisig workflow: incorrect 'Pending' label for a transaction that is actually ready to broadcast
No input validation or authorization changes in the diff
No cryptographic or signature-verification logic changes
Test-only addition: no advisory, CVE, or vendor security framing supplied
Evidence from the diff
PendingTransactionService.RefreshSignatureProgress now computes the effective PSBT, applies any signature-progress drift, and additionally checks whether a transaction in PendingTransactionState.Pending has collected enough signatures to finalize. If effectivePsbt.TryFinalize succeeds, it flips the state to Signed and persists the blob. The unit test was extended to simulate a ‘stale’ pending record with enough signatures and assert that GetPendingTransaction promotes it to Signed. Another test was adjusted to assert that an insufficiently-signed transaction remains Pending. The diff shows a functional fix, not a hardening patch, but the commit message frames it as routine finalization behavior.
Changed components
BTCPayServer/HostedServices/PendingTransactionService.csBTCPayServer.Tests/MultisigTests.csMultisig pending-transaction state trackingInspect captured patch +60 / −6
diff --git a/BTCPayServer.Tests/MultisigTests.cs b/BTCPayServer.Tests/MultisigTests.cs
index c6dc1bc..299a717 100644
--- a/BTCPayServer.Tests/MultisigTests.cs
+++ b/BTCPayServer.Tests/MultisigTests.cs
@@ -177,7 +177,7 @@ public class MultisigTests(ITestOutputHelper helper) : UnitTestBase(helper)
[Fact]
[Trait("Integration", "Integration")]
- public async Task PendingMultisigTransactionCountsFinalizedInputsAndSelfHealsStoredProgress()
+ public async Task PendingMultisigTransactionCountsFinalizedInputsAndSelfHealsStoredProgressAndState()
{
var dbTester = CreateDBTester();
await dbTester.MigrateAsync();
@@ -257,6 +257,39 @@ public class MultisigTests(ITestOutputHelper helper) : UnitTestBase(helper)
Assert.Equal(2, blob.SignaturesNeeded);
Assert.Equal(3, blob.SignaturesTotal);
Assert.Equal(1, blob.SignaturesCollected);
+
+ await using (var ctx = dbContextFactory.CreateContext())
+ {
+ var staleTransaction = await ctx.PendingTransactions.FindAsync(pendingTransaction.Id);
+ Assert.NotNull(staleTransaction);
+ var staleBlob = staleTransaction!.GetBlob();
+ staleBlob.CollectedSignatures.Add(new CollectedSignature
+ {
+ ReceivedPSBT = SignInputs(testPsbt.BasePsbt, testPsbt.SignerB, 1).ToBase64(),
+ Timestamp = DateTimeOffset.UtcNow
+ });
+ staleBlob.SignaturesNeeded = 2;
+ staleBlob.SignaturesTotal = 3;
+ staleBlob.SignaturesCollected = 2;
+ staleTransaction.SetBlob(staleBlob);
+ Assert.Equal(PendingTransactionState.Pending, staleTransaction.State);
+ await ctx.SaveChangesAsync();
+ }
+
+ var finalized = await pendingTransactionService.GetPendingTransaction(
+ new PendingTransactionService.PendingTransactionFullId("BTC", storeId, pendingTransaction.Id));
+ blob = finalized!.GetBlob();
+ Assert.Equal(2, blob.SignaturesNeeded);
+ Assert.Equal(3, blob.SignaturesTotal);
+ Assert.Equal(2, blob.SignaturesCollected);
+ Assert.Equal(PendingTransactionState.Signed, finalized.State);
+
+ await using (var ctx = dbContextFactory.CreateContext())
+ {
+ var persisted = await ctx.PendingTransactions.FindAsync(pendingTransaction.Id);
+ Assert.NotNull(persisted);
+ Assert.Equal(PendingTransactionState.Signed, persisted!.State);
+ }
}
[Fact]
@@ -365,6 +398,10 @@ public class MultisigTests(ITestOutputHelper helper) : UnitTestBase(helper)
entry.Level == LogLevel.Warning &&
entry.Message.Contains(pendingTransaction.Id, StringComparison.Ordinal));
+ pendingTransaction = await pendingTransactionService.GetPendingTransaction(pendingTransactionId);
+ Assert.NotNull(pendingTransaction);
+ Assert.Equal(PendingTransactionState.Pending, pendingTransaction.State);
+
// Per-input progress must be retained even while the aggregate remains at two.
pendingTransaction = await pendingTransactionService.CollectSignature(
pendingTransactionId,
diff --git a/BTCPayServer/HostedServices/PendingTransactionService.cs b/BTCPayServer/HostedServices/PendingTransactionService.cs
index fd96a40..9f093fd 100644
--- a/BTCPayServer/HostedServices/PendingTransactionService.cs
+++ b/BTCPayServer/HostedServices/PendingTransactionService.cs
@@ -349,13 +349,30 @@ public class PendingTransactionService(
if (network is null)
return false;
- var progress = GetSignatureProgress(BuildEffectivePsbt(blob, network));
- if (blob.SignaturesNeeded == progress.SignaturesNeeded &&
- blob.SignaturesTotal == progress.SignaturesTotal &&
- blob.SignaturesCollected == progress.SignaturesCollected)
+ var effectivePsbt = BuildEffectivePsbt(blob, network);
+ var progress = GetSignatureProgress(effectivePsbt);
+ var changed = blob.SignaturesNeeded != progress.SignaturesNeeded ||
+ blob.SignaturesTotal != progress.SignaturesTotal ||
+ blob.SignaturesCollected != progress.SignaturesCollected;
+
+ if (changed)
+ ApplyProgress(blob, progress);
+
+ if (pendingTransaction.State == PendingTransactionState.Pending &&
+ progress.SignaturesNeeded > 0 &&
+ progress.SignaturesCollected >= progress.SignaturesNeeded &&
+ effectivePsbt.TryFinalize(out _))
+ {
+ pendingTransaction.State = PendingTransactionState.Signed;
+ changed = true;
+ logger.LogInformation(
+ "Finalized pending transaction {PendingTransactionId} while refreshing stored signature progress",
+ pendingTransaction.Id);
+ }
+
+ if (!changed)
return false;
- ApplyProgress(blob, progress);
pendingTransaction.SetBlob(blob);
return true;
}
Why this scored 40/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.