👍 instagibbs approved a pull request: "doc: use TRUC instead of v3 and add release note"
(https://github.com/bitcoin/bitcoin/pull/30272#pullrequestreview-2126011000)
ACK a6821a0a4e1cf2aba679962289f4beeefd3d539a
fairly mechanical changes
(https://github.com/bitcoin/bitcoin/pull/30272#pullrequestreview-2126011000)
ACK a6821a0a4e1cf2aba679962289f4beeefd3d539a
fairly mechanical changes
💬 instagibbs commented on pull request "doc: use TRUC instead of v3 and add release note":
(https://github.com/bitcoin/bitcoin/pull/30272#discussion_r1644761817)
could directly link the BIP here: https://github.com/bitcoin/bips/blob/master/bip-0431.mediawiki
(https://github.com/bitcoin/bitcoin/pull/30272#discussion_r1644761817)
could directly link the BIP here: https://github.com/bitcoin/bips/blob/master/bip-0431.mediawiki
🤔 mzumsande reviewed a pull request: "chainparams: Add achow101 DNS seeder"
(https://github.com/bitcoin/bitcoin/pull/30007#pullrequestreview-2126027578)
ACK 2721d64989c2b2114890586b7efd01ab4b062ca6
(https://github.com/bitcoin/bitcoin/pull/30007#pullrequestreview-2126027578)
ACK 2721d64989c2b2114890586b7efd01ab4b062ca6
💬 Sjors commented on pull request "Introduce Mining interface":
(https://github.com/bitcoin/bitcoin/pull/30200#issuecomment-2176551584)
Rebased and added some missing `#include`s and `class`es.
(https://github.com/bitcoin/bitcoin/pull/30200#issuecomment-2176551584)
Rebased and added some missing `#include`s and `class`es.
💬 edilmedeiros commented on pull request "depends: bump miniupnpc to 2.2.8":
(https://github.com/bitcoin/bitcoin/pull/30301#issuecomment-2176563635)
Concept ACK.
From the CI output, this has to wait for #30283 to be merged first, right?
(https://github.com/bitcoin/bitcoin/pull/30301#issuecomment-2176563635)
Concept ACK.
From the CI output, this has to wait for #30283 to be merged first, right?
💬 Sjors commented on pull request "depends: bump miniupnpc to 2.2.8":
(https://github.com/bitcoin/bitcoin/pull/30301#issuecomment-2176564289)
cc @laanwj
(https://github.com/bitcoin/bitcoin/pull/30301#issuecomment-2176564289)
cc @laanwj
💬 edilmedeiros commented on pull request "upnp: fix build with miniupnpc 2.2.8":
(https://github.com/bitcoin/bitcoin/pull/30283#issuecomment-2176568646)
reACK 8acdf66540834b9f9cf28f16d389e8b6a48516d5
(https://github.com/bitcoin/bitcoin/pull/30283#issuecomment-2176568646)
reACK 8acdf66540834b9f9cf28f16d389e8b6a48516d5
💬 sipa commented on pull request "Several randomness improvements":
(https://github.com/bitcoin/bitcoin/pull/29625#issuecomment-2176619287)
Made the following changes:
```diff
diff --git a/src/random.cpp b/src/random.cpp
index 71a2fcc781..39a1e3eab9 100644
--- a/src/random.cpp
+++ b/src/random.cpp
@@ -763,5 +763,12 @@ void RandomInit()
double MakeExponentiallyDistributed(uint64_t uniform) noexcept
{
- return -std::log1p(uniform * -5.42101086242752217e-20 /* -1/2^64 */);
+ // To convert uniform into an exponentially-distributed double, we use two steps:
+ // - Convert uniform into a uniformly-distributed do
...
(https://github.com/bitcoin/bitcoin/pull/29625#issuecomment-2176619287)
Made the following changes:
```diff
diff --git a/src/random.cpp b/src/random.cpp
index 71a2fcc781..39a1e3eab9 100644
--- a/src/random.cpp
+++ b/src/random.cpp
@@ -763,5 +763,12 @@ void RandomInit()
double MakeExponentiallyDistributed(uint64_t uniform) noexcept
{
- return -std::log1p(uniform * -5.42101086242752217e-20 /* -1/2^64 */);
+ // To convert uniform into an exponentially-distributed double, we use two steps:
+ // - Convert uniform into a uniformly-distributed do
...
📝 am-sq opened a pull request: "doc: loadwallet loads from relative walletdir"
(https://github.com/bitcoin/bitcoin/pull/30302)
## Why this change?
https://github.com/bitcoin/bitcoin/issues/30269 describes a need for documentation improvement with the `loadwallet` RPC. Namely, [some users have found](https://bitcoin.stackexchange.com/questions/123331/how-do-you-load-a-regtest-wallet) the usage description confusing when it comes to loading wallets that are not in the normal case of being in the default wallet directory.
The default wallet directory, depending on the machine OS, has the base directory defined here:
...
(https://github.com/bitcoin/bitcoin/pull/30302)
## Why this change?
https://github.com/bitcoin/bitcoin/issues/30269 describes a need for documentation improvement with the `loadwallet` RPC. Namely, [some users have found](https://bitcoin.stackexchange.com/questions/123331/how-do-you-load-a-regtest-wallet) the usage description confusing when it comes to loading wallets that are not in the normal case of being in the default wallet directory.
The default wallet directory, depending on the machine OS, has the base directory defined here:
...
💬 maflcko commented on pull request "doc: loadwallet loads from relative walletdir":
(https://github.com/bitcoin/bitcoin/pull/30302#discussion_r1644847206)
This is wrong. Absolute paths are allowed.
(https://github.com/bitcoin/bitcoin/pull/30302#discussion_r1644847206)
This is wrong. Absolute paths are allowed.
💬 Sjors commented on pull request "contrib: add tool to convert compact-serialized UTXO set to SQLite database":
(https://github.com/bitcoin/bitcoin/pull/27432#issuecomment-2176675056)
TOTAL: 176948713 coins written to ../utxo-snapshots/utxo-840000.sqlite
Not sure if it's supposed to be deterministic, but here you go:
```
99450bac6e081e03751c064034b3d10e17b463ca56dfc35797fc6b5fe0de8ac4 ../utxo-snapshots/utxo-840000.sqlite
```
(https://github.com/bitcoin/bitcoin/pull/27432#issuecomment-2176675056)
TOTAL: 176948713 coins written to ../utxo-snapshots/utxo-840000.sqlite
Not sure if it's supposed to be deterministic, but here you go:
```
99450bac6e081e03751c064034b3d10e17b463ca56dfc35797fc6b5fe0de8ac4 ../utxo-snapshots/utxo-840000.sqlite
```
💬 am-sq commented on pull request "doc: loadwallet loads from relative walletdir":
(https://github.com/bitcoin/bitcoin/pull/30302#discussion_r1644867028)
Thanks for the review. Is this description better?
```
You may pass in the absolute path of your wallet directory or .dat file. Otherwise, the interpreted path will be relative to the default wallet directory.
```
(https://github.com/bitcoin/bitcoin/pull/30302#discussion_r1644867028)
Thanks for the review. Is this description better?
```
You may pass in the absolute path of your wallet directory or .dat file. Otherwise, the interpreted path will be relative to the default wallet directory.
```
💬 maflcko commented on pull request "doc: loadwallet loads from relative walletdir":
(https://github.com/bitcoin/bitcoin/pull/30302#discussion_r1644869283)
Do sqlite wallets allow `.dat` files to be passed, or is this only for BDB legacy wallets?
(https://github.com/bitcoin/bitcoin/pull/30302#discussion_r1644869283)
Do sqlite wallets allow `.dat` files to be passed, or is this only for BDB legacy wallets?
💬 am-sq commented on pull request "doc: loadwallet loads from relative walletdir":
(https://github.com/bitcoin/bitcoin/pull/30302#discussion_r1644870338)
I can also add examples that load wallet using an absolute path.
(https://github.com/bitcoin/bitcoin/pull/30302#discussion_r1644870338)
I can also add examples that load wallet using an absolute path.
💬 mzumsande commented on pull request "test/BIP324: disconnection scenarios during v2 handshake":
(https://github.com/bitcoin/bitcoin/pull/29431#discussion_r1644874521)
Removing the possibility of 0 garbage in `EncryptedP2PState` just to make this test nicer is a bit unfortunate in my opinion: If the node had a bug dealing with zero-length garbage, the test framework wouldn't be able to catch that anymore. Can't we change it locally just for the test cases who need it?
(https://github.com/bitcoin/bitcoin/pull/29431#discussion_r1644874521)
Removing the possibility of 0 garbage in `EncryptedP2PState` just to make this test nicer is a bit unfortunate in my opinion: If the node had a bug dealing with zero-length garbage, the test framework wouldn't be able to catch that anymore. Can't we change it locally just for the test cases who need it?
📝 theuni converted_to_draft a pull request: "depends: bump miniupnpc to 2.2.8"
(https://github.com/bitcoin/bitcoin/pull/30301)
Drops two of our patches that have been merged upstream and adjusts the other to deal with recent changes.
Follow-up from #30283. I can't vouch for the upstream changes here.
(https://github.com/bitcoin/bitcoin/pull/30301)
Drops two of our patches that have been merged upstream and adjusts the other to deal with recent changes.
Follow-up from #30283. I can't vouch for the upstream changes here.
💬 theuni commented on pull request "depends: bump miniupnpc to 2.2.8":
(https://github.com/bitcoin/bitcoin/pull/30301#issuecomment-2176713996)
Marked as draft #30283 is merged.
(https://github.com/bitcoin/bitcoin/pull/30301#issuecomment-2176713996)
Marked as draft #30283 is merged.
💬 mzumsande commented on pull request "assumeutxo: Check snapshot base block is not in invalid chain":
(https://github.com/bitcoin/bitcoin/pull/30267#discussion_r1644893828)
Looks good (except for the compiler issue with `Join()`) - I think it would be helpful to not only log the reason, but also return them with the RPC - for that, `ActivateSnapshot()` would need to return it, possibly as `util::Result` as suggested by ryanofsky, but that could be a follow-up.
Though for now, it might be helpful to expand the error ("Unable to load UTXO snapshot") to point the user to the debug log for the reason.
(https://github.com/bitcoin/bitcoin/pull/30267#discussion_r1644893828)
Looks good (except for the compiler issue with `Join()`) - I think it would be helpful to not only log the reason, but also return them with the RPC - for that, `ActivateSnapshot()` would need to return it, possibly as `util::Result` as suggested by ryanofsky, but that could be a follow-up.
Though for now, it might be helpful to expand the error ("Unable to load UTXO snapshot") to point the user to the debug log for the reason.
🤔 sipa reviewed a pull request: "net: Replace libnatpmp with built-in PCP+NATPMP implementation"
(https://github.com/bitcoin/bitcoin/pull/30043#pullrequestreview-2126279596)
Just a quick look so far.
(https://github.com/bitcoin/bitcoin/pull/30043#pullrequestreview-2126279596)
Just a quick look so far.
💬 sipa commented on pull request "net: Replace libnatpmp with built-in PCP+NATPMP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1644928807)
In commit "net: Add PCP and NATPMP implementation":
Nit: `check_packet({response, recvsz})` should work too (if no type is listed, the corresponding constructor of the declared type is used).
(https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1644928807)
In commit "net: Add PCP and NATPMP implementation":
Nit: `check_packet({response, recvsz})` should work too (if no type is listed, the corresponding constructor of the declared type is used).