💬 darosior commented on issue "RFC: multiprocess binaries in 29.0":
(https://github.com/bitcoin/bitcoin/issues/31756#issuecomment-2622845955)
Yes, i would say the same about another feature in the same stage of development.
(https://github.com/bitcoin/bitcoin/issues/31756#issuecomment-2622845955)
Yes, i would say the same about another feature in the same stage of development.
💬 theuni commented on pull request "cmake: Set top-level target output locations":
(https://github.com/bitcoin/bitcoin/pull/31161#issuecomment-2622853846)
@achow101 This is one of the changes I was referring to in last week's IRC meeting. We'd rather not move these around after v29 (with the exception of some moves to libexec) so that nobody has to migrate to CMake and then adapt again.
Not sure what you mean about breaking existing tooling though? This affects the location in the CMake build tree. It's not related to installation locations. So afaik the only tooling that needs to be updated has been changed in this PR. Presumably anyone else r
...
(https://github.com/bitcoin/bitcoin/pull/31161#issuecomment-2622853846)
@achow101 This is one of the changes I was referring to in last week's IRC meeting. We'd rather not move these around after v29 (with the exception of some moves to libexec) so that nobody has to migrate to CMake and then adapt again.
Not sure what you mean about breaking existing tooling though? This affects the location in the CMake build tree. It's not related to installation locations. So afaik the only tooling that needs to be updated has been changed in this PR. Presumably anyone else r
...
💬 achow101 commented on pull request "RPC: improve SFFO arg parsing, error catching and coverage":
(https://github.com/bitcoin/bitcoin/pull/30844#issuecomment-2622885360)
ACK cddcbaf81e8e3d107cd321ee2c9a7fd786a8917e
(https://github.com/bitcoin/bitcoin/pull/30844#issuecomment-2622885360)
ACK cddcbaf81e8e3d107cd321ee2c9a7fd786a8917e
💬 glozow commented on pull request "multi-peer orphan resolution followups":
(https://github.com/bitcoin/bitcoin/pull/31666#discussion_r1934672284)
I wonder if a good "mitigation" (if we're worried) is @mzumsande other suggestion, which is to process 1 thing off the peer's workset when it connects? Although if trying to "attack" this way, the solution then is to just have 2 in the workset. I'd propose the same trick then, which is randomly popping from workset. That outta be good enough? I prefer something like this to potentially re-validating repeatedly.
However I think it's very realistic to add outbound/inbound information to txorpha
...
(https://github.com/bitcoin/bitcoin/pull/31666#discussion_r1934672284)
I wonder if a good "mitigation" (if we're worried) is @mzumsande other suggestion, which is to process 1 thing off the peer's workset when it connects? Although if trying to "attack" this way, the solution then is to just have 2 in the workset. I'd propose the same trick then, which is randomly popping from workset. That outta be good enough? I prefer something like this to potentially re-validating repeatedly.
However I think it's very realistic to add outbound/inbound information to txorpha
...
🚀 achow101 merged a pull request: "RPC: improve SFFO arg parsing, error catching and coverage"
(https://github.com/bitcoin/bitcoin/pull/30844)
(https://github.com/bitcoin/bitcoin/pull/30844)
💬 achow101 commented on pull request "ci: optionally use local docker build cache":
(https://github.com/bitcoin/bitcoin/pull/31545#issuecomment-2622922911)
ACK e87429a2d0f23eb59526d335844fa5ff5b50b21f
(https://github.com/bitcoin/bitcoin/pull/31545#issuecomment-2622922911)
ACK e87429a2d0f23eb59526d335844fa5ff5b50b21f
🚀 achow101 merged a pull request: "ci: optionally use local docker build cache"
(https://github.com/bitcoin/bitcoin/pull/31545)
(https://github.com/bitcoin/bitcoin/pull/31545)
💬 ryanofsky commented on pull request "cmake: Set top-level target output locations":
(https://github.com/bitcoin/bitcoin/pull/31161#issuecomment-2622947958)
> ~0
>
> I've read through the conversation but I still don't understand why this change is beneficial?
The main thing I like about this change is that makes it easier to write scripts and navigate the build. Examples:
1. When you run `make WHATEVER`, `WHATEVER` will be built in `bin/` if it is an executable `lib/` if it is a binary. You don't have to go looking all over the tree for the `WHATEVER` you just built in `src/` or `src/bench/` or `src/test/` or `src/qt/` or `/src/qt/test` or
...
(https://github.com/bitcoin/bitcoin/pull/31161#issuecomment-2622947958)
> ~0
>
> I've read through the conversation but I still don't understand why this change is beneficial?
The main thing I like about this change is that makes it easier to write scripts and navigate the build. Examples:
1. When you run `make WHATEVER`, `WHATEVER` will be built in `bin/` if it is an executable `lib/` if it is a binary. You don't have to go looking all over the tree for the `WHATEVER` you just built in `src/` or `src/bench/` or `src/test/` or `src/qt/` or `/src/qt/test` or
...
💬 achow101 commented on pull request "test: fix intermittent timeout in p2p_1p1c_network.py":
(https://github.com/bitcoin/bitcoin/pull/31751#issuecomment-2622971189)
ACK 152a2dcdefa6ec744db5b106d5c0a8c5b8aca416
(https://github.com/bitcoin/bitcoin/pull/31751#issuecomment-2622971189)
ACK 152a2dcdefa6ec744db5b106d5c0a8c5b8aca416
💬 glozow commented on pull request "multi-peer orphan resolution followups":
(https://github.com/bitcoin/bitcoin/pull/31666#discussion_r1934744219)
> Also, making sure, are there any ways 1P1C are affected by the choice either way?
No because 1p1c doesn't happen through the workset stuff. We pick an orphan child and immediately process it.
(https://github.com/bitcoin/bitcoin/pull/31666#discussion_r1934744219)
> Also, making sure, are there any ways 1P1C are affected by the choice either way?
No because 1p1c doesn't happen through the workset stuff. We pick an orphan child and immediately process it.
✅ achow101 closed an issue: "test: intermittent issue in p2p_1p1c_network.py "
(https://github.com/bitcoin/bitcoin/issues/31721)
(https://github.com/bitcoin/bitcoin/issues/31721)
🚀 achow101 merged a pull request: "test: fix intermittent timeout in p2p_1p1c_network.py"
(https://github.com/bitcoin/bitcoin/pull/31751)
(https://github.com/bitcoin/bitcoin/pull/31751)
💬 glozow commented on pull request "multi-peer orphan resolution followups":
(https://github.com/bitcoin/bitcoin/pull/31666#discussion_r1934780097)
You're totally right that it's convoluted! I've changed the commit to just move all the parent inv logic to `OrphanResolutionCandidate` instead. We don't need 2 small helper functions where 1 always calls the other. I think this is even clearer, but let me know if you think I should still split it into `IsOrphanCandidate()` and `AddOrphanParentAnnouncements()`?
(https://github.com/bitcoin/bitcoin/pull/31666#discussion_r1934780097)
You're totally right that it's convoluted! I've changed the commit to just move all the parent inv logic to `OrphanResolutionCandidate` instead. We don't need 2 small helper functions where 1 always calls the other. I think this is even clearer, but let me know if you think I should still split it into `IsOrphanCandidate()` and `AddOrphanParentAnnouncements()`?
💬 glozow commented on pull request "multi-peer orphan resolution followups":
(https://github.com/bitcoin/bitcoin/pull/31666#discussion_r1934780175)
added
(https://github.com/bitcoin/bitcoin/pull/31666#discussion_r1934780175)
added
💬 glozow commented on pull request "multi-peer orphan resolution followups":
(https://github.com/bitcoin/bitcoin/pull/31666#discussion_r1934780698)
I've changed this to assert that `candidate_peers` matches `m_announcements[txhash]`.
(https://github.com/bitcoin/bitcoin/pull/31666#discussion_r1934780698)
I've changed this to assert that `candidate_peers` matches `m_announcements[txhash]`.
💬 glozow commented on pull request "multi-peer orphan resolution followups":
(https://github.com/bitcoin/bitcoin/pull/31666#discussion_r1934781088)
Left as is for now
(https://github.com/bitcoin/bitcoin/pull/31666#discussion_r1934781088)
Left as is for now
💬 polespinasa commented on pull request "rpc, logging: return "verificationprogress" of 1 when up to date":
(https://github.com/bitcoin/bitcoin/pull/31177#issuecomment-2623149705)
@sipa and @laanwj pinging you here as this PR is based on your comments on https://github.com/bitcoin/bitcoin/pull/31135
This has been tested on mainnet and seems to be working with the solution proposed:
```
bitcoin-cli -rpcport=8332 getblockchaininfo
{
"chain": "main",
"blocks": 881415,
"headers": 881415,
...
"verificationprogress": 1,
"initialblockdownload": false,
...
"warnings": [
"This is a pre-release test build - use at your own risk - do not use for mini
...
(https://github.com/bitcoin/bitcoin/pull/31177#issuecomment-2623149705)
@sipa and @laanwj pinging you here as this PR is based on your comments on https://github.com/bitcoin/bitcoin/pull/31135
This has been tested on mainnet and seems to be working with the solution proposed:
```
bitcoin-cli -rpcport=8332 getblockchaininfo
{
"chain": "main",
"blocks": 881415,
"headers": 881415,
...
"verificationprogress": 1,
"initialblockdownload": false,
...
"warnings": [
"This is a pre-release test build - use at your own risk - do not use for mini
...
💬 achow101 commented on pull request "cmake: Set top-level target output locations":
(https://github.com/bitcoin/bitcoin/pull/31161#issuecomment-2623151935)
> Not sure what you mean about breaking existing tooling though?
> I think it would break internal tooling not external tooling since this PR does not affect install or release locations. Earlier I suggested this could create symlinks in old locations for convenience and to prevent breakage. I don't think too many tools should actually break, but I do think this change could be annoying to developers, for example if you just update git and rebuild, and try to run the old binary because you do
...
(https://github.com/bitcoin/bitcoin/pull/31161#issuecomment-2623151935)
> Not sure what you mean about breaking existing tooling though?
> I think it would break internal tooling not external tooling since this PR does not affect install or release locations. Earlier I suggested this could create symlinks in old locations for convenience and to prevent breakage. I don't think too many tools should actually break, but I do think this change could be annoying to developers, for example if you just update git and rebuild, and try to run the old binary because you do
...
💬 fjahr commented on pull request "rpc: have getblocktemplate mintime account for timewarp":
(https://github.com/bitcoin/bitcoin/pull/31600#issuecomment-2623243798)
tACK e1676b08f7b0b9a6c8ed76e31f24faa03a3facc9
Reviewed the code and verified that the test change covers the new behavior.
> Did we have any blocks on mainnet in the last couple of years that would have violated the new mintime?
To answer my own question, I checked this with a small script and there were quite a few blocks before 400k that would have been below the new `mintime` but since then this has been a rare exception. Here is the full list of such blocks since 400k that I found (
...
(https://github.com/bitcoin/bitcoin/pull/31600#issuecomment-2623243798)
tACK e1676b08f7b0b9a6c8ed76e31f24faa03a3facc9
Reviewed the code and verified that the test change covers the new behavior.
> Did we have any blocks on mainnet in the last couple of years that would have violated the new mintime?
To answer my own question, I checked this with a small script and there were quite a few blocks before 400k that would have been below the new `mintime` but since then this has been a rare exception. Here is the full list of such blocks since 400k that I found (
...
💬 mzumsande commented on pull request "multi-peer orphan resolution followups":
(https://github.com/bitcoin/bitcoin/pull/31666#discussion_r1934886294)
> I wonder if a good "mitigation" (if we're worried) is @mzumsande other suggestion, which is to process 1 thing off the peer's workset when it disconnects? Although if trying to "attack" this way, the solution then is to just have 2 in the workset.
Another idea that would deal with that is to assign it just to the peer who provided the parent here (no randomness), and during disconnection we don't do any validation but pass all oustanding workset entries on to another random announcer, if an
...
(https://github.com/bitcoin/bitcoin/pull/31666#discussion_r1934886294)
> I wonder if a good "mitigation" (if we're worried) is @mzumsande other suggestion, which is to process 1 thing off the peer's workset when it disconnects? Although if trying to "attack" this way, the solution then is to just have 2 in the workset.
Another idea that would deal with that is to assign it just to the peer who provided the parent here (no randomness), and during disconnection we don't do any validation but pass all oustanding workset entries on to another random announcer, if an
...