This PR adds test coverage for PartiallySignedTransaction::Merge() divided into 3 tests:
1.Commit 0d21098e27dff9dc930a1d1ce9f72d82f37aef89 only merge PSBTs with same version: Lines (46-47) have no coverage prior to this PR. The test can be triggered with this mutant as an example:
<details> <summary>Diff</summary>
diff --git a/src/psbt.cpp b/src/psbt.cpp
index 51fb19591a..cf5eef0089 100644
--- a/src/psbt.cpp
+++ b/src/psbt.cpp
@@ -43,7 +43,7 @@ bool PartiallySignedTransaction::Merge(const PartiallySignedTransaction& psbt)
return false;
}
if (GetVersion() != psbt.GetVersion()) {
- return false;
+ return true;
}
for (unsigned int i = 0; i < inputs.size(); ++i) {
</details>
2.Commit 0aedfc5a5918fae10f26efccdbfc2e332c8ad458 merge PSBT fallback_locktime coverage: Line 56 checks if the psbt has a fallback locktime, if not, it adopts the one from the psbt is merging. Prior to this PR no coverage was provided. The test can be triggered with this mutant as an example:
<details> <summary>Diff</summary>
diff --git a/src/psbt.cpp b/src/psbt.cpp
index 51fb19591a..ce909eb062 100644
--- a/src/psbt.cpp
+++ b/src/psbt.cpp
@@ -53,7 +53,7 @@ bool PartiallySignedTransaction::Merge(const PartiallySignedTransaction& psbt)
outputs[i].Merge(psbt.outputs[i]);
}
MergeGlobalXPubs(psbt);
- if (fallback_locktime == std::nullopt && psbt.fallback_locktime != std::nullopt) fallback_locktime = psbt.fallback_locktime;
+ if (fallback_locktime == std::nullopt || psbt.fallback_locktime != std::nullopt) fallback_locktime = psbt.fallback_locktime;
// Set m_tx_modifiable only if either PSBT had it set
if (m_tx_modifiable.has_value() || psbt.m_tx_modifiable.has_value()) {
</details>
While writing this test I had a question: If both PSBTs have a fallback_locktime value, the "this" (receiving side) wins silently. Is this the expected behavior? Why not keep the max or directly fail merging if we have two conflicting fallback_locktimes?
3.Commit 4a5a082d306d9b4eab4ea9db6643efc307a9e961 merge PSBT m_tx_modifiable coverage: Added missing coverage for all the mutants. The test can be triggered with this mutant as one of the various examples:
<details> <summary>Diff</summary>
diff --git a/src/psbt.cpp b/src/psbt.cpp
index 51fb19591a..29eb910014 100644
--- a/src/psbt.cpp
+++ b/src/psbt.cpp
@@ -56,7 +56,7 @@ bool PartiallySignedTransaction::Merge(const PartiallySignedTransaction& psbt)
if (fallback_locktime == std::nullopt && psbt.fallback_locktime != std::nullopt) fallback_locktime = psbt.fallback_locktime;
// Set m_tx_modifiable only if either PSBT had it set
- if (m_tx_modifiable.has_value() || psbt.m_tx_modifiable.has_value()) {
+ if (m_tx_modifiable.has_value() && psbt.m_tx_modifiable.has_value()) {
// In general, we AND the modifiable flags
std::bitset<8> this_modifiable = m_tx_modifiable.value_or(0);
std::bitset<8> psbt_modifiable = psbt.m_tx_modifiable.value_or(0);
</details>
