💬 hebasto commented on pull request "depends: reuse _config_opts for CMake options":
(https://github.com/bitcoin/bitcoin/pull/27496#issuecomment-1516598966)
My Guix builds:
```
45d96de0248ec8b604cc5c980f1abbc780862d497f195a795c6088087d646c73 guix-build-63c0c4ff10b2/output/aarch64-linux-gnu/SHA256SUMS.part
ebdd087cbb45f1fa337b717c0d7bd53c8b7d0bc78d4e9693de2430a1ab9e03d9 guix-build-63c0c4ff10b2/output/aarch64-linux-gnu/bitcoin-63c0c4ff10b2-aarch64-linux-gnu-debug.tar.gz
e09206e6a70068036ea52731e0f8122665525a132e85f2798662ff9d7644dd8b guix-build-63c0c4ff10b2/output/aarch64-linux-gnu/bitcoin-63c0c4ff10b2-aarch64-linux-gnu.tar.gz
20e71234d2e90386
...
(https://github.com/bitcoin/bitcoin/pull/27496#issuecomment-1516598966)
My Guix builds:
```
45d96de0248ec8b604cc5c980f1abbc780862d497f195a795c6088087d646c73 guix-build-63c0c4ff10b2/output/aarch64-linux-gnu/SHA256SUMS.part
ebdd087cbb45f1fa337b717c0d7bd53c8b7d0bc78d4e9693de2430a1ab9e03d9 guix-build-63c0c4ff10b2/output/aarch64-linux-gnu/bitcoin-63c0c4ff10b2-aarch64-linux-gnu-debug.tar.gz
e09206e6a70068036ea52731e0f8122665525a132e85f2798662ff9d7644dd8b guix-build-63c0c4ff10b2/output/aarch64-linux-gnu/bitcoin-63c0c4ff10b2-aarch64-linux-gnu.tar.gz
20e71234d2e90386
...
💬 dergoegge commented on pull request "mempool / rpc: add getprioritisationmap, delete a mapDeltas entry when delta==0":
(https://github.com/bitcoin/bitcoin/pull/27501#issuecomment-1516600484)
Maybe add a short description for what `mapDeltas` is to the PR description under "motivation / background"? afaict `mapDeltas` also doesn't have any code documentation.
(https://github.com/bitcoin/bitcoin/pull/27501#issuecomment-1516600484)
Maybe add a short description for what `mapDeltas` is to the PR description under "motivation / background"? afaict `mapDeltas` also doesn't have any code documentation.
💬 ajtowns commented on pull request "Allow configuring target block time for a signet":
(https://github.com/bitcoin/bitcoin/pull/27446#issuecomment-1516600820)
> If we want to allow for more configurable signet consensus rules, then I think we need to do it in a more robust way
I think the easy, mostly backwards compatible way of doing this would just be to have the signetchallenge contain the data directly, eg making it be the script `OP_RETURN PUSHDATA<params> PUSHDATA<actual challenge>`. Then you're still just passing around a hex "challenge" string that completely defines the signet consensus behaviour, and will get different magic when there a
...
(https://github.com/bitcoin/bitcoin/pull/27446#issuecomment-1516600820)
> If we want to allow for more configurable signet consensus rules, then I think we need to do it in a more robust way
I think the easy, mostly backwards compatible way of doing this would just be to have the signetchallenge contain the data directly, eg making it be the script `OP_RETURN PUSHDATA<params> PUSHDATA<actual challenge>`. Then you're still just passing around a hex "challenge" string that completely defines the signet consensus behaviour, and will get different magic when there a
...
👍 MarcoFalke approved a pull request: "mempool / rpc: add getprioritisationmap, delete a mapDeltas entry when delta==0"
(https://github.com/bitcoin/bitcoin/pull/27501#pullrequestreview-1394327087)
lgtm ACK. Left two nits.
(https://github.com/bitcoin/bitcoin/pull/27501#pullrequestreview-1394327087)
lgtm ACK. Left two nits.
💬 MarcoFalke commented on pull request "mempool / rpc: add getprioritisationmap, delete a mapDeltas entry when delta==0":
(https://github.com/bitcoin/bitcoin/pull/27501#discussion_r1172806194)
clang-format new code (only if you retouch)
(https://github.com/bitcoin/bitcoin/pull/27501#discussion_r1172806194)
clang-format new code (only if you retouch)
💬 MarcoFalke commented on pull request "mempool / rpc: add getprioritisationmap, delete a mapDeltas entry when delta==0":
(https://github.com/bitcoin/bitcoin/pull/27501#discussion_r1172814908)
```suggestion
result.try_emplace(txid, delta, in_mempool);
```
nit: `try_emplace` should avoid duplicate and unneeded (move) constructor calls? Also, it is shorter.
(https://github.com/bitcoin/bitcoin/pull/27501#discussion_r1172814908)
```suggestion
result.try_emplace(txid, delta, in_mempool);
```
nit: `try_emplace` should avoid duplicate and unneeded (move) constructor calls? Also, it is shorter.
💬 achow101 commented on pull request "wallet: improve IBD sync time by skipping block scanning prior birth time":
(https://github.com/bitcoin/bitcoin/pull/27469#discussion_r1172817209)
> Hmm, wasn't the timestamp window created to extend the grace period enough so we can use the block time directly?.
AFAIK it exists for accepting blocks with future timestamps, and we only use it in the wallet because it's convenient.
But my comment isn't really about the accuracy of the timestamp, but rather about the possibility of skipping a block after we have already started scanning. A block's timestamp can be before its parent. The code as written would let us skip scanning a block
...
(https://github.com/bitcoin/bitcoin/pull/27469#discussion_r1172817209)
> Hmm, wasn't the timestamp window created to extend the grace period enough so we can use the block time directly?.
AFAIK it exists for accepting blocks with future timestamps, and we only use it in the wallet because it's convenient.
But my comment isn't really about the accuracy of the timestamp, but rather about the possibility of skipping a block after we have already started scanning. A block's timestamp can be before its parent. The code as written would let us skip scanning a block
...
💬 fanquake commented on issue "Document CoreDev organization":
(https://github.com/bitcoin/bitcoin/issues/27497#issuecomment-1516608574)
> Without a desire to make more noise here, just to clarify your position
There's no problem with having any discussion about how to organize a developer meetup, I just don't think having it in this issue tracker is the right place, or going to be particularly productive.
(https://github.com/bitcoin/bitcoin/issues/27497#issuecomment-1516608574)
> Without a desire to make more noise here, just to clarify your position
There's no problem with having any discussion about how to organize a developer meetup, I just don't think having it in this issue tracker is the right place, or going to be particularly productive.
💬 MarcoFalke commented on issue "meta: Isolated fuzzing of net processing":
(https://github.com/bitcoin/bitcoin/issues/27502#issuecomment-1516620806)
I don't think that #https://github.com/bitcoin/bitcoin/pull/27499 is going to increase fuzz performance. Parsing should currently only be done at the beginning once, before any fuzzing happens. Even if parsing were in the hot loop, it probably wouldn't be noticeable. And if you want to construct a PeerManager for each fuzzing iteration, it seems too slow either way, unless you find a way to skip the memory allocations (as you said yourself).
(https://github.com/bitcoin/bitcoin/issues/27502#issuecomment-1516620806)
I don't think that #https://github.com/bitcoin/bitcoin/pull/27499 is going to increase fuzz performance. Parsing should currently only be done at the beginning once, before any fuzzing happens. Even if parsing were in the hot loop, it probably wouldn't be noticeable. And if you want to construct a PeerManager for each fuzzing iteration, it seems too slow either way, unless you find a way to skip the memory allocations (as you said yourself).
💬 ariard commented on issue "Document CoreDev organization":
(https://github.com/bitcoin/bitcoin/issues/27497#issuecomment-1516640923)
Okay, I'll propose a document draft during the coming future to make things more productive. Sorry for the issue tracker usage, unclear where "meta-discussions" should go between the mailing list and here.
(https://github.com/bitcoin/bitcoin/issues/27497#issuecomment-1516640923)
Okay, I'll propose a document draft during the coming future to make things more productive. Sorry for the issue tracker usage, unclear where "meta-discussions" should go between the mailing list and here.
💬 dergoegge commented on issue "meta: Isolated fuzzing of net processing":
(https://github.com/bitcoin/bitcoin/issues/27502#issuecomment-1516641523)
> I don't think that #https://github.com/bitcoin/bitcoin/pull/27499 is going to increase fuzz performance.
I think the separation from gArgs makes sense either way, but yea not really a performance improvement. I added the args stuff because I was investigating performance for a target that creates a new TestingSetup each iteration. The argsman setup with all arguments actually was one of the slower things (among block tree db, chainman setup and blockfile creation).
> And if you want to
...
(https://github.com/bitcoin/bitcoin/issues/27502#issuecomment-1516641523)
> I don't think that #https://github.com/bitcoin/bitcoin/pull/27499 is going to increase fuzz performance.
I think the separation from gArgs makes sense either way, but yea not really a performance improvement. I added the args stuff because I was investigating performance for a target that creates a new TestingSetup each iteration. The argsman setup with all arguments actually was one of the slower things (among block tree db, chainman setup and blockfile creation).
> And if you want to
...
💬 MarcoFalke commented on pull request "rpc: Add importmempool RPC":
(https://github.com/bitcoin/bitcoin/pull/27460#discussion_r1172860400)
Done
(https://github.com/bitcoin/bitcoin/pull/27460#discussion_r1172860400)
Done
💬 MarcoFalke commented on pull request "rpc: Add importmempool RPC":
(https://github.com/bitcoin/bitcoin/pull/27460#discussion_r1172860609)
thx, added doc
(https://github.com/bitcoin/bitcoin/pull/27460#discussion_r1172860609)
thx, added doc
💬 TheCharlatan commented on pull request "refactor: Move chain constants to the util library":
(https://github.com/bitcoin/bitcoin/pull/27491#issuecomment-1516662993)
> scripted-diff to switch to ChainType::MAIN and args.GetChainType()
I don't think this is feasible, because we have to decide on the type (either calling a helper function for a string or the enum) based on the context, no?
(https://github.com/bitcoin/bitcoin/pull/27491#issuecomment-1516662993)
> scripted-diff to switch to ChainType::MAIN and args.GetChainType()
I don't think this is feasible, because we have to decide on the type (either calling a helper function for a string or the enum) based on the context, no?
🚀 achow101 merged a pull request: "p2p: update hardcoded mainnet seeds for 25.x"
(https://github.com/bitcoin/bitcoin/pull/27488)
(https://github.com/bitcoin/bitcoin/pull/27488)
🤔 ryanofsky reviewed a pull request: "RPC: Accept options as named-only parameters"
(https://github.com/bitcoin/bitcoin/pull/26485#pullrequestreview-1394502135)
Updated 284173937d156e1c0f291abe0b82274468d242e8 -> eaee226a17546b93245ca1435e4b468c368d9e86 ([`pr/nonly.13`](https://github.com/ryanofsky/bitcoin/commits/pr/nonly.13) -> [`pr/nonly.14`](https://github.com/ryanofsky/bitcoin/commits/pr/nonly.14), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/nonly.13..pr/nonly.14)) making check for duplicate parameter names less strict and adding more test coverage for the check
(https://github.com/bitcoin/bitcoin/pull/26485#pullrequestreview-1394502135)
Updated 284173937d156e1c0f291abe0b82274468d242e8 -> eaee226a17546b93245ca1435e4b468c368d9e86 ([`pr/nonly.13`](https://github.com/ryanofsky/bitcoin/commits/pr/nonly.13) -> [`pr/nonly.14`](https://github.com/ryanofsky/bitcoin/commits/pr/nonly.14), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/nonly.13..pr/nonly.14)) making check for duplicate parameter names less strict and adding more test coverage for the check
💬 ryanofsky commented on pull request "RPC: Accept options as named-only parameters":
(https://github.com/bitcoin/bitcoin/pull/26485#discussion_r1172920871)
re: https://github.com/bitcoin/bitcoin/pull/26485#discussion_r1172071849
> Otherwise a duplicate positional parameter after the `OBJ_NAMED_PARAMS` entry will give an error.
Agree that's not desirable. The reason I wrote it the other way though was that I was trying to prevent the code from allowing two `also_positional = true` parameters from having the same name. I updated the check to just do the right thing in both cases and added test coverage for both.
(https://github.com/bitcoin/bitcoin/pull/26485#discussion_r1172920871)
re: https://github.com/bitcoin/bitcoin/pull/26485#discussion_r1172071849
> Otherwise a duplicate positional parameter after the `OBJ_NAMED_PARAMS` entry will give an error.
Agree that's not desirable. The reason I wrote it the other way though was that I was trying to prevent the code from allowing two `also_positional = true` parameters from having the same name. I updated the check to just do the right thing in both cases and added test coverage for both.
💬 jamesob commented on pull request "Bump python minimum version to 3.8":
(https://github.com/bitcoin/bitcoin/pull/27483#issuecomment-1516742614)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/27483#issuecomment-1516742614)
Concept ACK
📝 achow101 opened a pull request: "Bump to 25.99 and remove release note fragments"
(https://github.com/bitcoin/bitcoin/pull/27503)
Pre-25.x branch off version bump and release note fragments removal.
The 25.0 draft release notes are in the dev wiki: https://github.com/bitcoin-core/bitcoin-devwiki/wiki/25.0-Release-Notes-Draft
(https://github.com/bitcoin/bitcoin/pull/27503)
Pre-25.x branch off version bump and release note fragments removal.
The 25.0 draft release notes are in the dev wiki: https://github.com/bitcoin-core/bitcoin-devwiki/wiki/25.0-Release-Notes-Draft
💬 jamesob commented on pull request "Bump python minimum version to 3.8":
(https://github.com/bitcoin/bitcoin/pull/27483#issuecomment-1516748540)
It looks like the commit message on https://github.com/bitcoin/bitcoin/pull/27483/commits/fa762d7f52db9c65df967f9cb22c6be963ce0300 is out of date, which suggests we're swiching from g++-8 to clang, but the contents of the commit suggest we're going to g++-9 instead.
(https://github.com/bitcoin/bitcoin/pull/27483#issuecomment-1516748540)
It looks like the commit message on https://github.com/bitcoin/bitcoin/pull/27483/commits/fa762d7f52db9c65df967f9cb22c6be963ce0300 is out of date, which suggests we're swiching from g++-8 to clang, but the contents of the commit suggest we're going to g++-9 instead.