💬 fjahr commented on pull request "test: Assumeutxo: import snapshot in a node with a divergent chain":
(https://github.com/bitcoin/bitcoin/pull/29996#discussion_r1644745285)
> After following you steps and connecting and syncing the nodes, the divergent chain is replaced with the original chain because has more work. If I don't run sync, it's not replaced, but I guess it's just a matter of time? or maybe I don't understand how connect and sync works.
When we see two blocks on the same height, we take the one that is seen first as the tip, not the one that has more work. We do this to ensure a miner who found a very low hash doesn't withhold it. What I described b
...
(https://github.com/bitcoin/bitcoin/pull/29996#discussion_r1644745285)
> After following you steps and connecting and syncing the nodes, the divergent chain is replaced with the original chain because has more work. If I don't run sync, it's not replaced, but I guess it's just a matter of time? or maybe I don't understand how connect and sync works.
When we see two blocks on the same height, we take the one that is seen first as the tip, not the one that has more work. We do this to ensure a miner who found a very low hash doesn't withhold it. What I described b
...
💬 fjahr commented on pull request "test: Assumeutxo: import snapshot in a node with a divergent chain":
(https://github.com/bitcoin/bitcoin/pull/29996#discussion_r1644749546)
Maybe rather check the chainstates instead if you want to add something more but I think it's not absolutely necessary.
(https://github.com/bitcoin/bitcoin/pull/29996#discussion_r1644749546)
Maybe rather check the chainstates instead if you want to add something more but I think it's not absolutely necessary.
💬 fjahr commented on pull request "test: Assumeutxo: import snapshot in a node with a divergent chain":
(https://github.com/bitcoin/bitcoin/pull/29996#discussion_r1644663646)
```suggestion
self.log.info('Ensuring background validation finishes')
```
(https://github.com/bitcoin/bitcoin/pull/29996#discussion_r1644663646)
```suggestion
self.log.info('Ensuring background validation finishes')
```
💬 fjahr commented on pull request "test: Assumeutxo: import snapshot in a node with a divergent chain":
(https://github.com/bitcoin/bitcoin/pull/29996#discussion_r1644672188)
This is a f-string now but its actually not needed here.
(https://github.com/bitcoin/bitcoin/pull/29996#discussion_r1644672188)
This is a f-string now but its actually not needed here.
👍 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.