Skip to content

Commit

Permalink
dsproof: When an orphan is claimed, clear the nodeId
Browse files Browse the repository at this point in the history
Summary
---

This fixes an issue described in Bitcoin-ABC#311 where a claimed dsproof orphan
could lead to a situation where the peer that told you about the proof
ends up being erroneously punished with a misbehavior score of +1.

The soluton is to clear the dsproof Entry "nodeId" back to -1 when we
accept a proof and associate it with a real mempool txn.  At this point
tracking the nodeId (which is only used for banning) is no longer needed
and it should be cleared to prevent banning later when the dsproof
completes its lifecycle. Ratonale: if a proof was ever accepeted for any
reason, banning should become impossible in the future.

Note: The normal lifecycle of a dsproof when its txn confirms is to always
become an orphan first (so that it cna be kept around in case of reorg),
before being deleted by the cleaner task after 90 seconds.

Test Plan
---

`ninja all check-all`
  • Loading branch information
cculianu committed May 20, 2021
1 parent b5b5bdf commit 3944570
Showing 1 changed file with 5 additions and 1 deletion.
6 changes: 5 additions & 1 deletion src/dsproof/storage.cpp
Expand Up @@ -44,7 +44,11 @@ bool DoubleSpendProofStorage::add(const DoubleSpendProof &proof)
if (it->orphan) {
// mark it as not an orphan now due to explicit add
decrementOrphans(1);
m_proofs.modify(it, [](Entry &e) { e.orphan = false; }, ModFastFail());
m_proofs.modify(it, [](Entry &e) {
e.orphan = false;
// we must clear the "bannable nodeId" here since we accepted this proof as good (see issue #311)
e.nodeId = -1;
}, ModFastFail());
}
return false;
}
Expand Down

0 comments on commit 3944570

Please sign in to comment.