Avoid using sizes on non-fixed-width types to derive protocol constants. #10854

pull gmaxwell wants to merge 1 commits into bitcoin:master from gmaxwell:rbf-numlimit-fix changing 2 files +5 −4

gmaxwell commented at 5:03 PM on July 17, 2017: contributor

Thanks to awemany for pointing this out.

This replaces #10172 which appears to be abandoned, but uses the constants as requested on that PR.
Avoid using sizes on non-fixed-width types to derive protocol constants.
Thanks to awemany for pointing this out.
095b917464

in src/validation.cpp:508 in 095b917464

 504 | @@ -504,7 +505,7 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool
 505 |                  {
 506 |                      for (const CTxIn &_txin : ptxConflicting->vin)
 507 |                      {
 508 | -                        if (_txin.nSequence < std::numeric_limits<unsigned int>::max()-1)
 509 | +                        if (_txin.nSequence <= MAX_BIP125_RBF_SEQUENCE)

laanwj commented at 5:18 PM on July 17, 2017:

MAX_BIP125_RBF_SEQUENCE is 0xfffffffd The old value was 0xfffffffe (on supported platforms) Is this intended? Edit: oh, <=, ok never mind

in src/validation.cpp:23 in 095b917464
```
  19 | @@ -20,6 +20,7 @@
  20 |  #include "init.h"
  21 |  #include "policy/fees.h"
  22 |  #include "policy/policy.h"
  23 | +#include "policy/rbf.h"
```
laanwj commented at 5:19 PM on July 17, 2017:

Do we want validation to depend on those policy headers? It seems the wrong way around.

TheBlueMatt commented at 5:29 PM on July 17, 2017:

ATMP does policy-based decisions. I think the real answer here is ATMP belongs outside of validation.cpp, but dear god thats a big refactor (I think).

gmaxwell commented at 11:04 PM on July 17, 2017:

I don't make the standards, I just follow them. (didn't think much about it because of the above two lines). :)

laanwj commented at 6:40 AM on July 18, 2017:

Well it would be wrong to rely on a policy constant if ATMP was a consensus critical function, but you're right.
TheBlueMatt commented at 5:42 PM on July 17, 2017: member

utACK 095b9174645f5b855dd5946c99cea4f4ffb5a034
fanquake added the label Validation on Jul 18, 2017
dcousens approved
laanwj commented at 6:48 AM on July 26, 2017: member

utACK 095b917
laanwj merged this on Jul 26, 2017
laanwj closed this on Jul 26, 2017
laanwj referenced this in commit 04d395e832 on Jul 26, 2017
laanwj referenced this in commit 1227be30ec on Aug 14, 2017
codablock referenced this in commit f7a6790623 on Sep 23, 2019
UdjinM6 referenced this in commit d6ca9a78ef on Sep 24, 2019
barrystyle referenced this in commit 4bf59cd785 on Jan 22, 2020
DrahtBot locked this on Sep 8, 2021

Contributors

Labels

Linked (view graph)

#10172 Fix opt-in RBF reliance on compiler integer size #10938 Build failed on Ubuntu 14.04LTS