Bitcoin Core Github
43 subscribers
122K links
Download Telegram
🚀 achow101 merged a pull request: "addrman, refactor: improve stochastic test in `AddSingle`"
(https://github.com/bitcoin/bitcoin/pull/27319)
💬 sr-gi commented on pull request "p2p: Allow whitelisting manual connections":
(https://github.com/bitcoin/bitcoin/pull/27114#discussion_r1483439607)
Ohh ok, that used to only be relevant to inbound but we are now also applying it to manual
maflcko closed an issue: "memcmp with constants that contain zero bytes are broken in GCC"
(https://github.com/bitcoin/bitcoin/issues/20005)
💬 maflcko commented on issue "memcmp with constants that contain zero bytes are broken in GCC":
(https://github.com/bitcoin/bitcoin/issues/20005#issuecomment-1934742874)
Closing for now. Feel free to reopen, but I don't think anything is going to be done about gcc 10.1 and 10.2 at this point.

See also https://github.com/bitcoin/bitcoin/pull/29091
maflcko closed an issue: "Flawed PROVIDE_FUZZ_MAIN_FUNCTION test"
(https://github.com/bitcoin/bitcoin/issues/28563)
💬 maflcko commented on issue "Flawed PROVIDE_FUZZ_MAIN_FUNCTION test":
(https://github.com/bitcoin/bitcoin/issues/28563#issuecomment-1934748470)
> I don't see any real practical harm in this

Let's move the discussion to https://github.com/bitcoin/bitcoin/pull/28564
📝 brunoerg opened a pull request: "fuzz: increase length of string used for `NetWhitelist{bind}Permissions::TryParse`"
(https://github.com/bitcoin/bitcoin/pull/29413)
The string `s` represents the value from `-whitelist`/`-whitebind` (e.g. "bloom,forcerelay,noban@1.2.3.4:32") and it is used in `NetWhitelistPermissions::TryParse` and `NetWhitebindPermissions::TryParse`. However, a max length of 32 is not enough to cover a lot of cases. Even disconsidering the permissions, 32 would not be enough to cover a lot of addresses. This PR fixes it.
maflcko closed an issue: "Unclear documentation about TX replacements in `gettransaction`"
(https://github.com/bitcoin/bitcoin/issues/27781)
👍 maflcko approved a pull request: "test, assumeutxo: Add test to ensure failure when mempool not empty"
(https://github.com/bitcoin/bitcoin/pull/29394#pullrequestreview-1871091239)
lgtm
💬 maflcko commented on pull request "test, assumeutxo: Add test to ensure failure when mempool not empty":
(https://github.com/bitcoin/bitcoin/pull/29394#discussion_r1483492912)
please don't remove the trailing comma
💬 maflcko commented on pull request "test, assumeutxo: Add test to ensure failure when mempool not empty":
(https://github.com/bitcoin/bitcoin/pull/29394#discussion_r1483493614)
why the extra_args juggling? Isn't this set by default already?
💬 sdaftuar commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#issuecomment-1934809271)
Updating my concept ACK. Reading through the numerous comments on this PR, the objections that have been raised have nothing to do with v3, and instead have to do with applications that might use it, and what their best design is. I think applications are free to optimize further, but this PR is not the place to do spec design of layer 2 protocols.

The way I see this issue is that RBF pinning due to the total fee requirement[^1] is a real issue for users, and has been since we first introd
...
💬 hernanmarino commented on pull request "test, assumeutxo: Add test to ensure failure when mempool not empty":
(https://github.com/bitcoin/bitcoin/pull/29394#issuecomment-1934821087)
Updated code with all suggestions.
💬 achow101 commented on pull request "wallet: batch erase procedures and improve 'EraseRecords' performance":
(https://github.com/bitcoin/bitcoin/pull/29403#discussion_r1483514809)
In 523525707742fc620039cd67e8f1437f7f613a01 "wallet: bdb batch 'ErasePrefix', do not create txn internally"

I think it would be relevant to mention that BDB returns errors on `del()` if it is not in a transaction.
💬 achow101 commented on pull request "wallet: batch erase procedures and improve 'EraseRecords' performance":
(https://github.com/bitcoin/bitcoin/pull/29403#discussion_r1483516605)
In 757d6516db56e2732ea77624e66025446cb816a4 "wallet: simplify EraseRecords by using 'ErasePrefix'"

This could be done in the commit introducing `RunWithinTxn()` rather than changing it in a later commit.
💬 achow101 commented on pull request "wallet: batch erase procedures and improve 'EraseRecords' performance":
(https://github.com/bitcoin/bitcoin/pull/29403#discussion_r1483514110)
In 523525707742fc620039cd67e8f1437f7f613a01 "wallet: bdb batch 'ErasePrefix', do not create txn internally"

nit: Could use `RunWithinTxn()`.
💬 achow101 commented on pull request "wallet: batch erase procedures and improve 'EraseRecords' performance":
(https://github.com/bitcoin/bitcoin/pull/29403#discussion_r1483515740)
In 757d6516db56e2732ea77624e66025446cb816a4 "wallet: simplify EraseRecords by using 'ErasePrefix'"

This overload feels unnecessary given that there are only 2 places this function is really used.
💬 achow101 commented on pull request "wallet: batch erase procedures and improve 'EraseRecords' performance":
(https://github.com/bitcoin/bitcoin/pull/29403#discussion_r1483514234)
In 523525707742fc620039cd67e8f1437f7f613a01 "wallet: bdb batch 'ErasePrefix', do not create txn internally"

nit: Could use `RunWithinTxn()`.
💬 jonatack commented on pull request "bitcoin-config.h includes cleanup":
(https://github.com/bitcoin/bitcoin/pull/29404#issuecomment-1934839236)
Concept ACK
💬 mzumsande commented on issue "wallet_reorgrestore.py failed":
(https://github.com/bitcoin/bitcoin/issues/29392#issuecomment-1934853492)
> If you can point me in the right direction

I don't really think anyone has actually managed to reproduce the failure (i.e. managed to get the test fail locally with master in the same way it did in the failed CI run, possibly after putting a sleep somewhere). Or if you have managed that, you should describe what exactly you did. Therefore, at this point I don't think anyone knows the right direction, and the main part of solving this issue - which is not necessarily easy - is figuring tha
...