Bitcoin Core Github
44 subscribers
120K links
Download Telegram
👍 ismaelsadeeq approved a pull request: "AcceptMultipleTransactions: Fix workspace not being set as client_maxfeerate failure"
(https://github.com/bitcoin/bitcoin/pull/29735#pullrequestreview-1986776879)
ACK https://github.com/bitcoin/bitcoin/pull/29735/commits/4fe7d150eb3c85a6597d8fc910fe1490358197ad

I've reviewed [73b68....4fe7d15](https://github.com/bitcoin/bitcoin/compare/73b68bd8b4f9447e30091c7f8c3dc91a086bd93b..4fe7d150eb3c85a6597d8fc910f)

---

I've Ran the fuzz test without f28338b82be9820380ef217905ae9c167164f181

And verified the crash

```terminal
Test unit written to ./crash-7fd49c80d3088f672b4bf03ab8a1d7f1aedff0aa
Base64: q6sAIqsA/6sACQCrAAAAANr/+zUBAQENASUABAB5aKqgnQAA
...
💬 ismaelsadeeq commented on pull request "AcceptMultipleTransactions: Fix workspace not being set as client_maxfeerate failure":
(https://github.com/bitcoin/bitcoin/pull/29735#discussion_r1556048973)
looking at this again, what do you think about just restarting the node with the `-datacarriersize=100000`, ` -maxmempool=5`, `-minrelaytxfee=0.00001000` after filling the mempool we then restart the node with default node settings?
Are there any advantage of delegating setting this to the caller?
If not then this will reduce duplication of all callers of `fill_mempool` settings this values.
🚀 fanquake merged a pull request: "clang-tidy: Enable misc-no-recursion"
(https://github.com/bitcoin/bitcoin/pull/29690)
🚀 fanquake merged a pull request: "Change Luke Dashjr seed to dashjr-list-of-p2p-nodes.us"
(https://github.com/bitcoin/bitcoin/pull/29691)
⚠️ fanquake opened an issue: "ci: failure in `rpc_scanblocks.py`"
(https://github.com/bitcoin/bitcoin/issues/29831)
https://github.com/bitcoin/bitcoin/actions/runs/8600455763/job/23565458580?pr=29707#step:7:3826:
```bash
node0 2024-04-08T13:15:12.710688Z [httpworker.3] [rpc/request.cpp:187] [parse] [rpc] ThreadRPCServer method=getindexinfo user=__cookie__
test 2024-04-08T13:15:12.711000Z TestFramework (ERROR): Assertion failed
Traceback (most recent call last):
File "/Users/runner/work/bitcoin/bitcoin/ci/scratch/build/bitcoin-x
...
👍 AngusP approved a pull request: "test: Makes `wait_for_getdata` delete data on checks, plus allows to check the getdata message type"
(https://github.com/bitcoin/bitcoin/pull/29748#pullrequestreview-1986851015)
reACK 067b009bbf47f7bc5adeb6b500042f7c44bfb03f
jonatack closed a pull request: "Do not log p2p bip61 reject messages, improve log, add tests"
(https://github.com/bitcoin/bitcoin/pull/28429)
💬 jonatack commented on pull request "p2p: peer connection bug fixes":
(https://github.com/bitcoin/bitcoin/pull/28248#issuecomment-2043167695)
Will continue work on these bugfixes. Closing to be able to re-open it.
jonatack closed a pull request: "p2p: peer connection bug fixes"
(https://github.com/bitcoin/bitcoin/pull/28248)
jonatack closed a pull request: "test: update tool_wallet coverage for unexpected writes to wallet"
(https://github.com/bitcoin/bitcoin/pull/28116)
💬 jonatack commented on pull request "test: update tool_wallet coverage for unexpected writes to wallet":
(https://github.com/bitcoin/bitcoin/pull/28116#issuecomment-2043168709)
Closing to be able to re-open it.
jonatack closed a pull request: "Severity-based logging -- parent PR"
(https://github.com/bitcoin/bitcoin/pull/25203)
💬 jonatack commented on pull request "Severity-based logging -- parent PR":
(https://github.com/bitcoin/bitcoin/pull/25203#issuecomment-2043169446)
Closing to be able to re-open it.
💬 jonatack commented on pull request "refactor: deduplicate AmountFromValue() functions":
(https://github.com/bitcoin/bitcoin/pull/28134#issuecomment-2043177731)
Will re-open in a new PR.
💬 jonatack commented on pull request "Fix logging RPC and -debugexclude with 0/none values, add test coverage, improve docs":
(https://github.com/bitcoin/bitcoin/pull/27231#issuecomment-2043185560)
Will re-open as a new PR, along with the alternative version, as it is a good and mature patch with many updates made for reviews and nits.
💬 davidgumberg commented on pull request "test: Update --tmpdir doc string to say directory must not exist":
(https://github.com/bitcoin/bitcoin/pull/29498#issuecomment-2043766965)
ACK https://github.com/bitcoin/bitcoin/pull/29498/commits/d4e36ae80d4b3f03647fd9057461edf7ecd794a3
🤔 tdb3 reviewed a pull request: "test: fix `keys_to_multisig_script` (P2MS) helper for n/k > 16"
(https://github.com/bitcoin/bitcoin/pull/28312#pullrequestreview-1987913814)
Great work catching the bug and making testing more robust.

ACK for d113709dc4e68535605d22814ea987b55469bd32.

One inline recommendation to cover an additional related edge case.
💬 tdb3 commented on pull request "test: fix `keys_to_multisig_script` (P2MS) helper for n/k > 16":
(https://github.com/bitcoin/bitcoin/pull/28312#discussion_r1556687636)
Since > 20 keys are not allowed when creating a multisig address, recommend adding a test case to check this.

Something like:
```diff
diff --git a/test/functional/rpc_createmultisig.py b/test/functional/rpc_createmultisig.py
index 4bbd2a526d..88f4e44a65 100755
--- a/test/functional/rpc_createmultisig.py
+++ b/test/functional/rpc_createmultisig.py
@@ -129,6 +129,9 @@ class RpcCreateMultiSigTest(BitcoinTestFramework):
res = self.nodes[0].createmultisig(nrequired=nkeys, keys=
...
💬 andrewtoth commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#issuecomment-2044108942)
Ran benchmarks to 800k blocks, 2 times each. As expected, results are very good when running pruned. When running non-pruned with default `dbcache`, however, `master` was 1% faster. I think this slowdown is negligible though and the improvements to syncing with pruning enabled make this worth the changes here. Also, potential improvements like https://github.com/bitcoin/bitcoin/pull/28945 will make up for it.

| | prune | dbcache | mean time | speedup |
|-----------:|----------:|------------
...
maflcko closed a pull request: "test: randomize perturbed files excluding ldb"
(https://github.com/bitcoin/bitcoin/pull/29182)