doc requirement that replacement must have higher feerate than direct conflicts #25382

1. 
   glozow commented at 4:37 pm on June 15, 2022: member

   RBF policy requires the replacement transaction have a higher feerate than each of the directly conflicting transactions (see PaysMoreThanConflicts). It was pointed out that this rule is undocumented: #25038 (review)
2. in test/functional/feature_rbf.py:723 in dedfdda7b3 outdated

   719@@ -716,5 +720,43 @@ def test_replacement_relay_fee(self):
   720         tx.vout[0].nValue -= 1
   721         assert_raises_rpc_error(-26, "insufficient fee", self.nodes[0].sendrawtransaction, tx.serialize().hex())
   722 
   723+    def test_feerate_rule(self):
   

   ---

   ---

   

   sdaftuar commented at 4:40 pm on June 15, 2022:

   Does the test at line 274 (test_replacement_feeperkb) not already cover this case?

   ---

   

   glozow commented at 4:46 pm on June 15, 2022:

   I guess this test is slightly better since it doesn’t use -acceptnonstdtxn ? 😅 I can delete it though, I mostly wrote it for my own understanding.

   ---

   

   sdaftuar commented at 4:48 pm on June 15, 2022:

   I mean I don’t have a view either way on adding more tests or replacing a test with a better one, just wanted to know if I was missing something! :)
3. laanwj added the label Docs on Jun 15, 2022
4. fanquake requested review from instagibbs on Jun 15, 2022
5. fanquake requested review from darosior on Jun 15, 2022
6. fanquake requested review from t-bast on Jun 15, 2022
7. 
   instagibbs commented at 6:50 pm on June 15, 2022: member

   ACK on text, probably don’t want duplicate tests though
8. 
   DrahtBot commented at 6:54 pm on June 15, 2022: contributor

   The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

   Conflicts

   Reviewers, this pull request conflicts with the following ones:

   • #25353 (Add a -fullrbf node setting by ariard)
   • #22867 (test: Extend test coverage of BIP125 and document confusing/inconsistent behavior by mjdietzx)

   If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

9. [doc] RBF feerate rule 2224bcabc4
10. glozow force-pushed on Jun 15, 2022
11. w0xlt approved
12. 
    w0xlt commented at 7:34 pm on June 15, 2022: contributor

    ACK https://github.com/bitcoin/bitcoin/pull/25382/commits/2224bcabc432ecaadd7cadda90ab41f0f455432c
13. 
    laanwj commented at 8:38 pm on June 15, 2022: member

    ACK 2224bcabc432ecaadd7cadda90ab41f0f455432c Might want to update the PR title for removing the test.
14. 
    sdaftuar commented at 8:42 pm on June 15, 2022: member

    ACK
15. 
    darosior commented at 9:27 pm on June 15, 2022: member

    ACK 2224bcabc432ecaadd7cadda90ab41f0f455432c

    Should have catched it in #23711 :/

16. in doc/policy/mempool-replacements.md:54 in 2224bcabc4

    50@@ -51,6 +51,13 @@ other consensus and policy rules, each of the following conditions are met:
    51    significant portions of the node's mempool using replacements with multiple directly conflicting
    52    transactions, each with large descendant sets.
    53 
    54+6. The replacement transaction's feerate is greater than the feerates of all directly conflicting
    

    ---

    ---

    

    ariard commented at 0:18 am on June 16, 2022:

    super-nit: strictly greater :)

    I think some L2s stacks are duplicating a bunch of mempools standardness checks in their broadcast backend (at least LDK do so). Allowing to issue replacement transactions which are feerate valid against <= and you’re Game Over (tm). So better than developers understand really well the mempool checks.

    (Please don’t update the PR for that, not necessary to invalidate previous ACKs)

17. ariard approved
18. 
    ariard commented at 0:19 am on June 16, 2022: member

    ACK 2224bcab
19. t-bast approved
20. 
    t-bast commented at 6:34 am on June 16, 2022: contributor

    ACK https://github.com/bitcoin/bitcoin/pull/25382/commits/2224bcabc432ecaadd7cadda90ab41f0f455432c
21. fanquake renamed this:
    doc and test requirement that replacement must have higher feerate than direct conflicts
    doc requirement that replacement must have higher feerate than direct conflicts
    on Jun 16, 2022
22. fanquake merged this on Jun 16, 2022
23. fanquake closed this on Jun 16, 2022

    ---

24. sidhujag referenced this in commit 457143b0d8 on Jun 16, 2022
25. fanquake added this to the milestone 24.0 on Sep 15, 2022
26. bitcoin locked this on Sep 15, 2023

Contributors
glozow sdaftuar instagibbs DrahtBot w0xlt laanwj darosior ariard t-bast

Review Requested
instagibbs

Labels
Docs

Milestone
24.0