💬 vasild commented on issue "RPC wont bind without an IP address on a non-localhost interface":
(https://github.com/bitcoin/bitcoin/issues/13155#issuecomment-2167939959)
> one of the two default rpcbinds (:: and 0.0.0.0) will fail
Which one? Is it always `::` or always `0.0.0.0` or randomly one or the other? I guess there is some mistake here because the default rpcbinds are `::1` and `127.0.0.1` (not `::` and `0.0.0.0`).
(https://github.com/bitcoin/bitcoin/issues/13155#issuecomment-2167939959)
> one of the two default rpcbinds (:: and 0.0.0.0) will fail
Which one? Is it always `::` or always `0.0.0.0` or randomly one or the other? I guess there is some mistake here because the default rpcbinds are `::1` and `127.0.0.1` (not `::` and `0.0.0.0`).
💬 tdb3 commented on pull request "test: Added test coverage to listsinceblock rpc":
(https://github.com/bitcoin/bitcoin/pull/30195#discussion_r1639777921)
Yeah, looks like we'd get a `FileNotFoundError` (induced with the code below, `blk_dat_moved.rename()` throws).
An assert to check that `blk_dat_moved.exists()` before `blk_dat_moved.rename()` would be overkill, so not needed.
```python
# Restoring block file
blk_dat_moved.unlink()
assert not blk_dat_moved.exists()
blk_dat_moved.rename(self.nodes[0].blocks_path / "blk00000.dat")
assert blk_dat.exists()
```
(https://github.com/bitcoin/bitcoin/pull/30195#discussion_r1639777921)
Yeah, looks like we'd get a `FileNotFoundError` (induced with the code below, `blk_dat_moved.rename()` throws).
An assert to check that `blk_dat_moved.exists()` before `blk_dat_moved.rename()` would be overkill, so not needed.
```python
# Restoring block file
blk_dat_moved.unlink()
assert not blk_dat_moved.exists()
blk_dat_moved.rename(self.nodes[0].blocks_path / "blk00000.dat")
assert blk_dat.exists()
```
💬 vasild commented on pull request "fuzz: FuzzedSock::Recv() don't lose bytes from MSG_PEEK read":
(https://github.com/bitcoin/bitcoin/pull/30273#discussion_r1639787804)
This assert is too strong! Removed and adjusted the code afterwards to expect that `m_peek_data` may contain some bytes.
Btw, I ran `FUZZ=i2p ./src/test/fuzz/fuzz` for some time and it did not crash, maybe I did not run it long enough.
(https://github.com/bitcoin/bitcoin/pull/30273#discussion_r1639787804)
This assert is too strong! Removed and adjusted the code afterwards to expect that `m_peek_data` may contain some bytes.
Btw, I ran `FUZZ=i2p ./src/test/fuzz/fuzz` for some time and it did not crash, maybe I did not run it long enough.
💬 vasild commented on pull request "fuzz: FuzzedSock::Recv() don't lose bytes from MSG_PEEK read":
(https://github.com/bitcoin/bitcoin/pull/30273#issuecomment-2167983116)
`f4793257cd...4d81b4de33`: rebase and resolve https://github.com/bitcoin/bitcoin/pull/30273#discussion_r1639721875
(https://github.com/bitcoin/bitcoin/pull/30273#issuecomment-2167983116)
`f4793257cd...4d81b4de33`: rebase and resolve https://github.com/bitcoin/bitcoin/pull/30273#discussion_r1639721875
⚠️ ryanofsky opened an issue: "RFC: Assumeutxo and large forks and reorgs"
(https://github.com/bitcoin/bitcoin/issues/30288)
(tl;dr This is mainly a question about when an assumeutxo snapshot is loaded, if it makes more sense for the original chainstate to continue downloading and attaching blocks in the normal way, or if it should only download and attach blocks leading up to the snapshot block.)
---
It seems unclear how validation code should behave when an assumeutxo snapshot is loaded, and then new headers are received pointing to a forked chain with more proof of work than any known chain including the snap
...
(https://github.com/bitcoin/bitcoin/issues/30288)
(tl;dr This is mainly a question about when an assumeutxo snapshot is loaded, if it makes more sense for the original chainstate to continue downloading and attaching blocks in the normal way, or if it should only download and attach blocks leading up to the snapshot block.)
---
It seems unclear how validation code should behave when an assumeutxo snapshot is loaded, and then new headers are received pointing to a forked chain with more proof of work than any known chain including the snap
...
💬 marcofleon commented on pull request "fuzz: FuzzedSock::Recv() don't lose bytes from MSG_PEEK read":
(https://github.com/bitcoin/bitcoin/pull/30273#discussion_r1639813563)
Nice, I'll retest.
> Btw, I ran `FUZZ=i2p ./src/test/fuzz/fuzz` for some time and it did not crash, maybe I did not run it long enough.
Interesting, mine crashed very quickly. Did you run it with the I2P dictionary?
(https://github.com/bitcoin/bitcoin/pull/30273#discussion_r1639813563)
Nice, I'll retest.
> Btw, I ran `FUZZ=i2p ./src/test/fuzz/fuzz` for some time and it did not crash, maybe I did not run it long enough.
Interesting, mine crashed very quickly. Did you run it with the I2P dictionary?
💬 willcl-ark commented on pull request "rename TransactionErrors: MISSING_INPUTS and ALREADY_IN_CHAIN":
(https://github.com/bitcoin/bitcoin/pull/30212#issuecomment-2168023304)
Rebased on master to fix merge conflict.
Added an extra commit to cover `PSBTError::MISSING_INPUTS`.
(https://github.com/bitcoin/bitcoin/pull/30212#issuecomment-2168023304)
Rebased on master to fix merge conflict.
Added an extra commit to cover `PSBTError::MISSING_INPUTS`.
💬 sipa commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#issuecomment-2168024936)
I have split off the optimizations for candidate search to PR #30286, and the merging & postprocessing algorithms to PR #30285, and renamed the PR. It is now focused on just adding the `Linearize()` function, with its eventual interface (including passing in an old linearization, and a randomization seed), but without optimizations beyond that.
(https://github.com/bitcoin/bitcoin/pull/30126#issuecomment-2168024936)
I have split off the optimizations for candidate search to PR #30286, and the merging & postprocessing algorithms to PR #30285, and renamed the PR. It is now focused on just adding the `Linearize()` function, with its eventual interface (including passing in an old linearization, and a randomization seed), but without optimizations beyond that.
💬 willcl-ark commented on pull request "Multiprocess bitcoin":
(https://github.com/bitcoin/bitcoin/pull/10102#issuecomment-2168028486)
Wow, sounds sounds like exactly the bug I hit @ryanofsky, great debugging; sorry I couldn't provide any followup for you sooner.
I'll rebase my `ipc + -ipcconnect` branch on this one, update my `libmultiprocess` installation, and see how it goes from there.
Thanks!
(https://github.com/bitcoin/bitcoin/pull/10102#issuecomment-2168028486)
Wow, sounds sounds like exactly the bug I hit @ryanofsky, great debugging; sorry I couldn't provide any followup for you sooner.
I'll rebase my `ipc + -ipcconnect` branch on this one, update my `libmultiprocess` installation, and see how it goes from there.
Thanks!
⚠️ sipa opened an issue: "Cluster mempool tracking issue"
(https://github.com/bitcoin/bitcoin/issues/30289)
What to review next:
* #30126
Plan:
* Generic utilities:
* [x] bitset: #30160
* [x] vecdeque: #30161
* Linearization algorithms:
* [ ] cluster linearization: #30126
* [ ] optimized candidate search (follow-up to 30126): #30286
* [ ] merging and postprocessing of linearizations (depends on 30126): #30285
* Cluster mempool implementation:
* [ ] #28676
(https://github.com/bitcoin/bitcoin/issues/30289)
What to review next:
* #30126
Plan:
* Generic utilities:
* [x] bitset: #30160
* [x] vecdeque: #30161
* Linearization algorithms:
* [ ] cluster linearization: #30126
* [ ] optimized candidate search (follow-up to 30126): #30286
* [ ] merging and postprocessing of linearizations (depends on 30126): #30285
* Cluster mempool implementation:
* [ ] #28676
🤔 glozow reviewed a pull request: "Cluster size 2 package rbf"
(https://github.com/bitcoin/bitcoin/pull/28984#pullrequestreview-2118444350)
reACK 94ed4fbf8e via range-diff
(https://github.com/bitcoin/bitcoin/pull/28984#pullrequestreview-2118444350)
reACK 94ed4fbf8e via range-diff
🤔 murchandamus reviewed a pull request: "bumpfee: ignore WALLET_INCREMENTAL_RELAY_FEE when user specifies fee_rate"
(https://github.com/bitcoin/bitcoin/pull/27969#pullrequestreview-2118458522)
ACK f58beabe754363cb7d5b24032fd392654b9514ac
(https://github.com/bitcoin/bitcoin/pull/27969#pullrequestreview-2118458522)
ACK f58beabe754363cb7d5b24032fd392654b9514ac
💬 tdb3 commented on pull request "netbase: extend CreateSock() to support creating arbitrary sockets":
(https://github.com/bitcoin/bitcoin/pull/30202#issuecomment-2168123034)
re ACK 1245d1388b003c46092937def7041917aecec8de
(https://github.com/bitcoin/bitcoin/pull/30202#issuecomment-2168123034)
re ACK 1245d1388b003c46092937def7041917aecec8de
👍 tdb3 approved a pull request: "netbase: extend CreateSock() to support creating arbitrary sockets"
(https://github.com/bitcoin/bitcoin/pull/30202#pullrequestreview-2118480160)
re ACK 1245d1388b003c46092937def7041917aecec8de
(https://github.com/bitcoin/bitcoin/pull/30202#pullrequestreview-2118480160)
re ACK 1245d1388b003c46092937def7041917aecec8de
💬 AngusP commented on pull request "test: Added test coverage to listsinceblock rpc":
(https://github.com/bitcoin/bitcoin/pull/30195#discussion_r1639929977)
Ah, d'oh. Full path is fine then, sorry I should've checked with a `Path` not in my current working dir
(https://github.com/bitcoin/bitcoin/pull/30195#discussion_r1639929977)
Ah, d'oh. Full path is fine then, sorry I should've checked with a `Path` not in my current working dir
👍 ismaelsadeeq approved a pull request: "Cluster size 2 package rbf"
(https://github.com/bitcoin/bitcoin/pull/28984#pullrequestreview-2118667559)
re-ACK 94ed4fbf8e1a396c650b5134d396d6c0be35ce10
(https://github.com/bitcoin/bitcoin/pull/28984#pullrequestreview-2118667559)
re-ACK 94ed4fbf8e1a396c650b5134d396d6c0be35ce10
💬 mzumsande commented on pull request "p2p: For assumeutxo, download snapshot chain before background chain":
(https://github.com/bitcoin/bitcoin/pull/29519#discussion_r1640012446)
I have now added a first commit that avoids downloading blocks not on the snapshot chain completely.
Local testing revealed that not only are we unable to reorg to a more-work chain, we also won't fail gracefully: On master, we would still download the blocks and attempt to reorg, but then the node would just crash.
(https://github.com/bitcoin/bitcoin/pull/29519#discussion_r1640012446)
I have now added a first commit that avoids downloading blocks not on the snapshot chain completely.
Local testing revealed that not only are we unable to reorg to a more-work chain, we also won't fail gracefully: On master, we would still download the blocks and attempt to reorg, but then the node would just crash.
👍 ismaelsadeeq approved a pull request: "rename TransactionErrors: MISSING_INPUTS and ALREADY_IN_CHAIN"
(https://github.com/bitcoin/bitcoin/pull/30212#pullrequestreview-2118694519)
re-ACK b2b4b932572c5bb1ffa3fc4f34e17130348fbc24
(https://github.com/bitcoin/bitcoin/pull/30212#pullrequestreview-2118694519)
re-ACK b2b4b932572c5bb1ffa3fc4f34e17130348fbc24
💬 mzumsande commented on pull request "p2p: For assumeutxo, download snapshot chain before background chain":
(https://github.com/bitcoin/bitcoin/pull/29519#issuecomment-2168303545)
[ac547ea ](https://github.com/bitcoin/bitcoin/commit/ac547ea43beb1d66d2c11e7e8abb76dd1bfc2883)to [e977c69](https://github.com/bitcoin/bitcoin/commit/e977c698f97ac9bba30e4e3837f41721841c28c4):
Added a commit that avoids downloading blocks not on the snapshot chain (see discussion above).
Also made small changes to the (now) second commit using `snap_base` instead of `GetSnapshotBaseHeight()`, and reworked the documentation.
(https://github.com/bitcoin/bitcoin/pull/29519#issuecomment-2168303545)
[ac547ea ](https://github.com/bitcoin/bitcoin/commit/ac547ea43beb1d66d2c11e7e8abb76dd1bfc2883)to [e977c69](https://github.com/bitcoin/bitcoin/commit/e977c698f97ac9bba30e4e3837f41721841c28c4):
Added a commit that avoids downloading blocks not on the snapshot chain (see discussion above).
Also made small changes to the (now) second commit using `snap_base` instead of `GetSnapshotBaseHeight()`, and reworked the documentation.
👍 theStack approved a pull request: "Cluster size 2 package rbf"
(https://github.com/bitcoin/bitcoin/pull/28984#pullrequestreview-2118754168)
Code-review ACK 94ed4fbf8e1a396c650b5134d396d6c0be35ce10
(https://github.com/bitcoin/bitcoin/pull/28984#pullrequestreview-2118754168)
Code-review ACK 94ed4fbf8e1a396c650b5134d396d6c0be35ce10