Changes in #12048 cause a compilation error in Arena::walk() when
ARENA_DEBUG is defined. Specifically, Arena's chunks_free map was
changed to have a different value type.
Additionally, missing includes cause other compilation errors when
ARENA_DEBUG is defined.
Reproduced with:
make CPPFLAGS=-DARENA_DEBUG
18b18f8e81 [build] ./configure --enable-werror: add unused-variable (Sjors Provoost)
Pull request description:
The two macOS Travis machines run with `--enable-werror`. This PR adds `-Werror=unused-variable` to the existing `vla`, `switch` and `thread-safety-analysis` checks. This should prevent the need for fixes like b07b07cd87, 26a93bce29, dd777f3e12, 99be644966, fa39f674ae, 16bcc1b823, bb079a0e2c, bdaed47558 and ecf9b25a03 with minimal nuisance.
Thoughts for followups:
* Travis starts these macOS machines fairly late, so we should consider setting `--enable-werror` on earlier machines as well.
* We should encourage the use of `--enable-werror` by developers. Maybe switch it on by default for `--enable-debug`?
* See practicalswift's overview of other checks to consider in #17344
ACKs for top commit:
MarcoFalke:
ACK 18b18f8e81
practicalswift:
ACK 18b18f8e81 -- nice!
Tree-SHA512: 892b471ca5ea547f3c952ac88190cbebf8110cb7aec6f20466aeb312aeb0910bfe990f914e153c40ecb55709c03775ef30770412ad76f9d532ca77055596c582
5e8a56348b test: add unit test for non-standard txs with too large scriptSig (Sebastian Falbesoner)
Pull request description:
Approaches the first missing test of issue #17394: Checks that the function `IsStandardTx()` returns rejection reason `"scriptsig-size"` if any one the inputs' scriptSig is larger than 1650 bytes.
ACKs for top commit:
MarcoFalke:
ACK 5e8a56348b
instagibbs:
ACK 5e8a56348b
Tree-SHA512: 79977b12ddea9438a37cefdbb48cc551e4ad02a8ccfaa2d2837ced9f3a185e2e07cc366c243b9e3c7736245e90e315d7b4110efc6b440c63dbef7ee2c9d78a73
38516f9078 Fix input size assertion in wallet_bumpfee.py (Gregory Sanders)
Pull request description:
I was investigating a curious error for https://github.com/bitcoin/bitcoin/pull/17290 and realized that this check should have caught that error earlier in the test.
The loop is intended to ensure that only a single input exists the entire time until the change output disappears, a single additional bump occurs, then it leaves the loop.
Top commit has no ACKs.
Tree-SHA512: 1d2d6ef535ec2c55f516ee5de11352386ceac6bedaabc6842229a486d9f28d35310ad5f57bfcc1f1e654fc397ecff29ec33256f9b3da897500b7e1635004b63a
-BEGIN VERIFY SCRIPT-
# tx pool member access (mempool followed by dot)
sed --regexp-extended -i -e 's/(::)?\<mempool\>\.([a-zA-Z])/m_node.mempool->\2/g' $(git grep -l mempool ./src/test)
# plain global (mempool not preceeded by dot, but followed by comma)
sed --regexp-extended -i -e 's/([^\.])(::)?\<mempool\>,/\1*m_node.mempool,/g' $(git grep -l mempool ./src/test)
-END VERIFY SCRIPT-
Currently it is an alias to the global ::mempool and should be used as
follows.
* Node code (validation and transaction relay) can use either ::mempool
or node.mempool, whichever seems a better fit.
* RPC code should use the added convenience getter EnsureMempool, which
makes sure the mempool exists before use. This prepares the RPC code
to a future where the mempool might be disabled at runtime or compile
time.
* Test code should use m_node.mempool directly, as the mempool is always
initialized for tests.
edb6b768a4 fix uninitialized variable nMinerConfirmationWindow (NullFunctor)
Pull request description:
It is used for the computation of `BIP9WarningHeight`, and by that time it isn't initialized.
ACKs for top commit:
jnewbery:
utACK edb6b768a
promag:
ACK edb6b768a4, commit description could be cleaned up though.
MarcoFalke:
ACK edb6b768a4, used python3 to do the addition locally 📍
practicalswift:
ACK edb6b768a4, used `clang++ -O2` on the previous version^W^W^W^W^W^W`bc` to verify the addition locally 🏓
Sjors:
Code review ACK edb6b76. Nit: commit description has duplicate text.
Tree-SHA512: 6fa0be0ecfbfd5d537f2c5b4a9333c76530c1f3182f777330cc7939b0496e37b75d8f8810cdaf471a9bd3247b425f2e239578300dfa0d5a87cd14a6ccfafa619
Also rename the "result_complete" variable in GetSettingsList() to "done" to be
more consistent with GetSetting().
This change doesn't affect current behavior but could be useful in the future
to support dynamically changing settings at runtime and adding new settings
sources, because it lets high priority sources reset settings back to default
(see test).
By removing a special case for null, this change also helps merge code treat
settings values more like black boxes, and interfere less with settings parsing
and retrieval.
333362991c doc: Explain $LIB in LD_PRELOAD in gitian descriptors (MarcoFalke)
fab9850ef4 scripted-diff: Avoid hardcoded libfaketime dir in gitian (MarcoFalke)
Pull request description:
Without this gitian prints warnings for me:
```
ERROR: ld.so: object '/usr/lib/x86_64-linux-gnu/faketime/libfaketime.so.1' from LD_PRELOAD cannot be preloaded (cannot open shared object file): ignored.
```
```
$ ls /usr/lib/aarch64-linux-gnu/faketime/libfaketime.so.1
/usr/lib/aarch64-linux-gnu/faketime/libfaketime.so.1
```
ACKs for top commit:
laanwj:
ACK 333362991c
Tree-SHA512: 3e7c4471b69c2ae38c29d0cc0db8b9eae0912085299d7f5ac67eeb4b6a2fdc7eb23d806eeeae0b0c2da22d6d1ba82513cab23652876b97aada9928b2c7d38e7e
The bool/int/string flags were added speculatively in #16097 and trigger errors
when type checking is actually implemented in
https://github.com/bitcoin/bitcoin/pull/16545
-BEGIN VERIFY SCRIPT-
sed -i 's/ALLOW_\(BOOL\|INT\|STRING\)/ALLOW_ANY/g' src/test/util_tests.cpp src/test/getarg_tests.cpp
-END VERIFY SCRIPT-
This commit does not change behavior.
Test GetSetting and GetArg type coercion, negation, and default value handling.
Test is expanded later to cover other flags besides ALLOW_ANY when they are
implemented in https://github.com/bitcoin/bitcoin/pull/16545
This commit does not change behavior.
Add for consistency with ArgsManager::GetSetting method and to make setting
types accessible to ArgsManager callers and tests (test added next commit).
This commit does not change behavior.
* Instead of calling RandAddSeedSleep anytime the scheduler goes
idle, call its replacement (RandAddSeedPeriodic) just once per
minute. This has better guarantees of actually being run, and
helps limit how frequently the dynamic env data is gathered.
* Since this code runs once per minute regardless now, we no
longer need to keep track of the last time strengthening was
run; just do it always.
* Make strengthening time context dependent (100 ms at startup,
10 ms once per minute afterwards).
49997813a4 test: check custom ancestor limit in mempool_packages.py (Sebastian Falbesoner)
Pull request description:
The functional test `mempool_packages.py` starts one node with default ancestor/descendant limit settings and one with a custom, reduced ancestor limit (currently `-limitancestorcount=5`). The effect of the latter had not been tested yet though. This is approached in this PR by checking on the expected mempool contents of node1 after the node0 ancestor tests are done, via the following three conditions:
- the # of txs in the node1 mempool is equal to the the limit
- all txs in node1 mempool are a subset of txs in node0 mempool
- the node1 mempool txs match the start of the constructed tx-chain
Note that this still doesn't *fully* check the expected mempool of node1 (e.g. that it isn't influenced by `prioritisetransaction` RPC on node0), hence I add another TODO. In the future it would make sense to also set a custom descendant limit when the second TODO about checking node1's mempool is approached: 89e93135ae/test/functional/mempool_packages.py (L228)
ACKs for top commit:
MarcoFalke:
ACK 49997813a4👲
Tree-SHA512: d3a1d19fb49731238ad08ee7c02e2fa81a227e3b4ef3340d68598de42ddb62be9161134f6b8e08fa76b8c9faa02fecfa01111159642e20e9f358292a757b7608