← Watch feed
Bound pending multisig signature history
Public commit record
50/100 · Thin
What the developer wrote
Authored by rockstardev
Bound pending multisig signature history
✓ Descriptive subject✓ Names a concrete action or component✓ Names security-relevant behavior explicitly! No meaningful explanatory body
What changed, and why it matters
This change tightens how BTCPay Server stores signature attempts for multi-signature Bitcoin transactions. Previously, every submitted PSBT (a partially signed transaction file) was kept in history, even if it added no real progress. Now only PSBTs that actually advance signing or successfully finalize the transaction are retained. This prevents unbounded growth of stored signature history, which could waste storage and potentially be abused to clutter or inflate a pending transaction record.
Recommended action
Review whether the retention gate (meaningfulDelta || finalized) is sufficient for all multisig policies and audit production pending transactions that may have accumulated large CollectedSignatures blobs. Consider adding a release note about the behavior change for operators relying on full PSBT history.
Security signals we found
01
Unbounded history/list growth bounded by logical retention gate
02
Resource consumption / storage abuse vector mitigated
03
Logic change in multi-signature signature collection path
04
Test expectations reduced for retained signature count
Evidence from the diff
The patch modifies PendingTransactionService.CollectSignature to compute signature progress before and after merging a newly submitted PSBT, then decides whether to retain the received PSBT based on meaningfulDelta or successful finalization. It reorders BuildEffectivePsbt and Combine so progress is measured correctly, only appends to blob.CollectedSignatures when retained is true, and updates the return flag and log message accordingly. Tests are adjusted to expect fewer retained entries when duplicate or non-progressing PSBTs are submitted.
Changed components
BTCPayServer/HostedServices/PendingTransactionService.csBTCPayServer.Tests/MultisigTests.csInspect captured patch +32 / −24
diff --git a/BTCPayServer.Tests/MultisigTests.cs b/BTCPayServer.Tests/MultisigTests.cs
index b2a0804..105c38e 100644
--- a/BTCPayServer.Tests/MultisigTests.cs
+++ b/BTCPayServer.Tests/MultisigTests.cs
@@ -122,10 +122,10 @@ public class MultisigTests(ITestOutputHelper helper) : UnitTestBase(helper)
signerAPartial,
CancellationToken.None);
blob = pendingTransaction!.GetBlob();
- Assert.Equal(2, blob.CollectedSignatures.Count);
+ Assert.Single(blob.CollectedSignatures);
Assert.Equal(1, blob.SignaturesCollected);
Assert.Equal(PendingTransactionState.Pending, pendingTransaction.State);
- Assert.Equal(2, signatureCollectedEvents);
+ Assert.Equal(1, signatureCollectedEvents);
var signerBFirstInput = SignInputs(testPsbt.BasePsbt, testPsbt.SignerB, 0);
pendingTransaction = await pendingTransactionService.CollectSignature(
@@ -133,10 +133,10 @@ public class MultisigTests(ITestOutputHelper helper) : UnitTestBase(helper)
signerBFirstInput,
CancellationToken.None);
blob = pendingTransaction!.GetBlob();
- Assert.Equal(3, blob.CollectedSignatures.Count);
+ Assert.Equal(2, blob.CollectedSignatures.Count);
Assert.Equal(1, blob.SignaturesCollected);
Assert.Equal(PendingTransactionState.Pending, pendingTransaction.State);
- Assert.Equal(3, signatureCollectedEvents);
+ Assert.Equal(2, signatureCollectedEvents);
var signerBSecondInput = SignInputs(testPsbt.BasePsbt, testPsbt.SignerB, 1);
pendingTransaction = await pendingTransactionService.CollectSignature(
@@ -144,11 +144,11 @@ public class MultisigTests(ITestOutputHelper helper) : UnitTestBase(helper)
signerBSecondInput,
CancellationToken.None);
blob = pendingTransaction!.GetBlob();
- Assert.Equal(4, blob.CollectedSignatures.Count);
+ Assert.Equal(3, blob.CollectedSignatures.Count);
Assert.Equal(2, blob.SignaturesCollected);
Assert.Equal(0, Math.Max(0, (blob.SignaturesNeeded ?? 0) - (blob.SignaturesCollected ?? 0)));
Assert.Equal(PendingTransactionState.Signed, pendingTransaction.State);
- Assert.Equal(4, signatureCollectedEvents);
+ Assert.Equal(3, signatureCollectedEvents);
var secondPendingTransaction = await pendingTransactionService.CreatePendingTransaction(
storeId,
@@ -260,7 +260,7 @@ public class MultisigTests(ITestOutputHelper helper) : UnitTestBase(helper)
[Fact]
[Trait("Integration", "Integration")]
- public async Task PendingTwoOfFourMultisigRetainsDistinctPSBTsAndRetriesFinalization()
+ public async Task PendingTwoOfFourMultisigRetainsProgressingPSBTsAndRetriesFinalization()
{
var dbTester = CreateDBTester();
await dbTester.MigrateAsync();
@@ -287,23 +287,24 @@ public class MultisigTests(ITestOutputHelper helper) : UnitTestBase(helper)
signerAAllInputs,
CancellationToken.None);
- // This PSBT is distinct, but adds no signature progress. Retain it and retry finalization anyway.
+ // This PSBT is distinct, but adds no signature progress. Retry finalization without retaining it.
var signerAFirstInput = SignInputs(testPsbt.BasePsbt, testPsbt.SignerA, 0);
pendingTransaction = await pendingTransactionService.CollectSignature(
pendingTransactionId,
signerAFirstInput,
CancellationToken.None);
var blob = pendingTransaction!.GetBlob();
- Assert.Equal(2, blob.CollectedSignatures.Count);
+ Assert.Single(blob.CollectedSignatures);
Assert.Equal(1, blob.SignaturesCollected);
- // Repeating the exact same PSBT is still ignored.
+ // Another distinct, non-progressing PSBT must not grow the retained history either.
+ var signerASecondInput = SignInputs(testPsbt.BasePsbt, testPsbt.SignerA, 1);
pendingTransaction = await pendingTransactionService.CollectSignature(
pendingTransactionId,
- signerAFirstInput,
+ signerASecondInput,
CancellationToken.None);
blob = pendingTransaction!.GetBlob();
- Assert.Equal(2, blob.CollectedSignatures.Count);
+ Assert.Single(blob.CollectedSignatures);
// Simulate the reported 2/4 Pending state: the second signature claims an eligible
// pubkey, but its signature cannot finalize the transaction.
@@ -315,6 +316,7 @@ public class MultisigTests(ITestOutputHelper helper) : UnitTestBase(helper)
invalidFourthSigner,
CancellationToken.None);
blob = pendingTransaction!.GetBlob();
+ Assert.Equal(2, blob.CollectedSignatures.Count);
Assert.Equal(2, blob.SignaturesCollected);
Assert.Equal(PendingTransactionState.Pending, pendingTransaction.State);
@@ -325,7 +327,7 @@ public class MultisigTests(ITestOutputHelper helper) : UnitTestBase(helper)
SignInputs(testPsbt.BasePsbt, testPsbt.SignerC, 0, 1),
CancellationToken.None);
blob = pendingTransaction!.GetBlob();
- Assert.Equal(4, blob.CollectedSignatures.Count);
+ Assert.Equal(3, blob.CollectedSignatures.Count);
Assert.Equal(3, blob.SignaturesCollected);
Assert.Equal(PendingTransactionState.Signed, pendingTransaction.State);
}
diff --git a/BTCPayServer/HostedServices/PendingTransactionService.cs b/BTCPayServer/HostedServices/PendingTransactionService.cs
index f00ba15..ad502d2 100644
--- a/BTCPayServer/HostedServices/PendingTransactionService.cs
+++ b/BTCPayServer/HostedServices/PendingTransactionService.cs
@@ -197,28 +197,34 @@ public class PendingTransactionService(
return (pendingTransaction, false);
}
- var beforeProgress = GetSignatureProgress(BuildEffectivePsbt(blob, network));
- var mergedPsbt = BuildEffectivePsbt(blob, network, psbt);
+ var mergedPsbt = BuildEffectivePsbt(blob, network);
+ var beforeProgress = GetSignatureProgress(mergedPsbt);
+ mergedPsbt.Combine(psbt);
var afterProgress = GetSignatureProgress(mergedPsbt);
var meaningfulDelta = HasMeaningfulDelta(beforeProgress, afterProgress);
- blob.CollectedSignatures.Add(new CollectedSignature
+ var finalized = mergedPsbt.TryFinalize(out var finalizationErrors);
+ var retained = meaningfulDelta || finalized;
+ if (retained)
{
- ReceivedPSBT = newPsbtBase64,
- Timestamp = DateTimeOffset.UtcNow
- });
+ blob.CollectedSignatures.Add(new CollectedSignature
+ {
+ ReceivedPSBT = newPsbtBase64,
+ Timestamp = DateTimeOffset.UtcNow
+ });
+ }
ApplyProgress(blob, afterProgress);
logger.LogInformation(
- "Retrying finalization for pending transaction {PendingTransactionId} after collecting PSBT {CollectedPSBTCount}. " +
- "Signature progress: {SignaturesCollected}/{SignaturesNeeded}; progress changed: {SignatureProgressChanged}",
+ "Retried finalization for pending transaction {PendingTransactionId}. Retained PSBT count: {CollectedPSBTCount}; " +
+ "signature progress: {SignaturesCollected}/{SignaturesNeeded}; PSBT retained: {PSBTRetained}",
pendingTransaction.Id,
blob.CollectedSignatures.Count,
blob.SignaturesCollected,
blob.SignaturesNeeded,
- meaningfulDelta);
+ retained);
- if (mergedPsbt.TryFinalize(out var finalizationErrors))
+ if (finalized)
{
if ((blob.SignaturesCollected ?? 0) < (blob.SignaturesNeeded ?? 0))
blob.SignaturesCollected = blob.SignaturesNeeded;
@@ -260,7 +266,7 @@ public class PendingTransactionService(
}
}
pendingTransaction.SetBlob(blob);
- return (pendingTransaction, true);
+ return (pendingTransaction, retained);
}
Risk score
Our methodology →Why this scored 35/100
Human-validated context
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
No validated notes yet.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.