diff options
author | fanquake <fanquake@gmail.com> | 2022-08-22 10:33:12 +0100 |
---|---|---|
committer | fanquake <fanquake@gmail.com> | 2022-08-22 10:35:26 +0100 |
commit | c5f0cbefa369b0e4d99a4f871e6334955d537c1f (patch) | |
tree | f1cc643164f8d5e2b9ed828a6554663f8f7711c8 /test/functional | |
parent | 607d5a46aa0f5053d8643a3e2c31a69bfdeb6e9f (diff) | |
parent | 1dc03dda05e9dce128e57f05bb7b1bb02b3cfb9e (diff) | |
download | bitcoin-c5f0cbefa369b0e4d99a4f871e6334955d537c1f.tar.xz |
Merge bitcoin/bitcoin#25775: docs: remove non-signaling mentions of BIP125
1dc03dda05e9dce128e57f05bb7b1bb02b3cfb9e [doc] remove non-signaling mentions of BIP125 (glozow)
32024d40f03fbf47c64d814fa5f2c2a73ec14cb7 scripted-diff: remove mention of BIP125 from non-signaling var names (glozow)
Pull request description:
We have pretty thorough documentation of our RBF policy in doc/policy/mempool-replacements.md. It enumerates each rule with several sentences of rationale. Also, each rule pretty much has its own function (3 and 4 share one), with extensive comments. The doc states explicitly that our rules are similar but differ from BIP125, and contains a record of historical changes to RBF policy.
We should not use "BIP125" as synonymous with our RBF policy because:
- Our RBF policy is different from what is specified in BIP125, for example:
- the BIP does not mention our rule about the replacement feerate being higher (our Rule 6)
- the BIP uses minimum relay feerate for Rule 4, while we have used incremental relay feerate since #9380
- the "inherited signaling" question (CVE-2021-31876). Call it discrepancy, ambiguous wording, doc misinterpretation, or implementation details, I would recommend users refer to doc/policy/mempool-replacements.md
- the signaling policy is configurable, see #25353
- Our RBF policy may change further
- We have already marked BIP125 as only "partially implemented" in docs/bips.md since 1fd49eb498c75a1d14193bb736d195a3dc75ae12
- See comments from people who are not me recently:
- https://github.com/bitcoin/bitcoin/pull/25038#discussion_r909507429
- https://github.com/bitcoin/bitcoin/pull/25575#issuecomment-1179519204
This PR removes all non-signaling mentions of BIP125 (if people feel strongly, we can remove all mentions of BIP125 period). It may be useful to refer to the concept of "tx opts in to RBF if it has at least one nSequence less than (0xffffffff - 1)" as "BIP125 signaling" because:
- It is succint.
- It has already been widely marketed as BIP125 opt-in signaling.
- Our API uses it when referring to signaling (e.g. getmempoolentry["bip125-replaceable"] and wallet error message "not BIP 125 replaceable"). Changing those is more invasive.
- If/when we have other ways to signal in the future, we can disambiguate them this way. See #25038 which proposes another way of signaling, and where I pulled these commits from.
Alternatives:
- Changing our policy to match BIP125. This doesn't make sense as, for example, we would have to remove the requirement that a replacement tx has a higher feerate (Rule 6).
- Changing BIP125 to match what we have. This doesn't make sense as it would be a significant change to a BIP years after it was finalized and already used as a spec to implement RBF in other places.
- Document our policy as a new BIP and give it a number. This might make sense if we don't expect things to change a lot, and can be done as a next step.
ACKs for top commit:
darosior:
ACK 1dc03dda05e9dce128e57f05bb7b1bb02b3cfb9e
ariard:
ACK 1dc03dda
t-bast:
ACK https://github.com/bitcoin/bitcoin/commit/1dc03dda05e9dce128e57f05bb7b1bb02b3cfb9e
Tree-SHA512: a3adc2039ec5785892d230ec442e50f47f7062717392728152bbbe27ce1c564141f85253143f53cb44e1331cf47476d74f5d2f4b3cd873fc3433d7a0aa783e02
Diffstat (limited to 'test/functional')
-rwxr-xr-x | test/functional/feature_rbf.py | 4 |
1 files changed, 2 insertions, 2 deletions
diff --git a/test/functional/feature_rbf.py b/test/functional/feature_rbf.py index 2237a4171e..7603248ae5 100755 --- a/test/functional/feature_rbf.py +++ b/test/functional/feature_rbf.py @@ -386,7 +386,7 @@ class ReplaceByFeeTest(BitcoinTestFramework): def test_too_many_replacements_with_default_mempool_params(self): """ - Test rule 5 of BIP125 (do not allow replacements that cause more than 100 + Test rule 5 (do not allow replacements that cause more than 100 evictions) without having to rely on non-default mempool parameters. In order to do this, create a number of "root" UTXOs, and then hang @@ -405,7 +405,7 @@ class ReplaceByFeeTest(BitcoinTestFramework): # limit; 10 works. num_tx_graphs = 10 - # (Number of transactions per graph, BIP125 rule 5 failure expected) + # (Number of transactions per graph, rule 5 failure expected) cases = [ # Test the base case of evicting fewer than MAX_REPLACEMENT_LIMIT # transactions. |