π¬ andremralves commented on pull request "contrib: skip missing binaries in gen-manpages":
(https://github.com/bitcoin/bitcoin/pull/30986#issuecomment-2440525890)
I probably donβt have enough understanding of the release process to know exactly how this change could affect it, but I assume that having gen-manpages.py exit with an error is important somewhere during the process.
I implemented your suggestion by adding the --skip-missing-binaries option to the command.
Thanks for the feedback! What do you think of it now?
(https://github.com/bitcoin/bitcoin/pull/30986#issuecomment-2440525890)
I probably donβt have enough understanding of the release process to know exactly how this change could affect it, but I assume that having gen-manpages.py exit with an error is important somewhere during the process.
I implemented your suggestion by adding the --skip-missing-binaries option to the command.
Thanks for the feedback! What do you think of it now?
π¬ stratospher commented on pull request "net: option to disallow v1 connection on ipv4 and ipv6 peers":
(https://github.com/bitcoin/bitcoin/pull/30951#issuecomment-2440608399)
I've rebased and changed the approach in the PR to have v2 only outbound connections. So there's null partition risk because of the option now.
> Right. Would it maybe make sense to have an option to restrict transaction broadcast and relay to v2-only, for IPv4 and IPv6 connections? Instead of not connect at all? Eg consider v1 connections blocks-only (except those over Tor/I2P). This alleviates the risk of network partition of consensus. While still providing a bit of added privacy for where
...
(https://github.com/bitcoin/bitcoin/pull/30951#issuecomment-2440608399)
I've rebased and changed the approach in the PR to have v2 only outbound connections. So there's null partition risk because of the option now.
> Right. Would it maybe make sense to have an option to restrict transaction broadcast and relay to v2-only, for IPv4 and IPv6 connections? Instead of not connect at all? Eg consider v1 connections blocks-only (except those over Tor/I2P). This alleviates the risk of network partition of consensus. While still providing a bit of added privacy for where
...
π¬ maflcko commented on issue "Stop at header":
(https://github.com/bitcoin/bitcoin/issues/31162#issuecomment-2440755070)
Does the hidden debug-only `-minimumchainwork=0` not work for this use case?
(https://github.com/bitcoin/bitcoin/issues/31162#issuecomment-2440755070)
Does the hidden debug-only `-minimumchainwork=0` not work for this use case?
:lock: fanquake locked an issue: "BTC"
(https://github.com/bitcoin/bitcoin/issues/31170)
(https://github.com/bitcoin/bitcoin/issues/31170)
π¬ maflcko commented on pull request "test: Don't enforce BIP94 on regtest unless specified by arg":
(https://github.com/bitcoin/bitcoin/pull/31156#discussion_r1818507761)
nit: Why add a new option at all? I wonder if using the existing `-test=<option>` hook would work.
(https://github.com/bitcoin/bitcoin/pull/31156#discussion_r1818507761)
nit: Why add a new option at all? I wonder if using the existing `-test=<option>` hook would work.
π¬ maflcko commented on pull request "guix: Use DOS newlines for SHA256SUMS files":
(https://github.com/bitcoin/bitcoin/pull/29147#issuecomment-2440840378)
Closing for now due to lack of progress. Leave a comment if you want this reopened.
(https://github.com/bitcoin/bitcoin/pull/29147#issuecomment-2440840378)
Closing for now due to lack of progress. Leave a comment if you want this reopened.
β
maflcko closed a pull request: "guix: Use DOS newlines for SHA256SUMS files"
(https://github.com/bitcoin/bitcoin/pull/29147)
(https://github.com/bitcoin/bitcoin/pull/29147)
π¬ Muaytie23 commented on issue "MIN_STANDARD_TX_NONWITNESS_SIZE prevents efficient spending of P2A outputs":
(https://github.com/bitcoin/bitcoin/issues/31155#issuecomment-2440891137)
[{"inputs":[{"internalType":"address","name":"_SY","type":"address"},{"internalType":"string","name":"_name","type":"string"},{"internalType":"string","name":"_symbol","type":"string"},{"internalType":"uint8","name":"__decimals","type":"uint8"},{"internalType":"uint256","name":"_expiry","type":"uint256"}],"stateMutability":"nonpayable","type":"constructor"},{"inputs":[],"name":"InvalidShortString","type":"error"},{"inputs":[],"name":"OnlyYCFactory","type":"error"},{"inputs":[],"name":"OnlyYT","t
...
(https://github.com/bitcoin/bitcoin/issues/31155#issuecomment-2440891137)
[{"inputs":[{"internalType":"address","name":"_SY","type":"address"},{"internalType":"string","name":"_name","type":"string"},{"internalType":"string","name":"_symbol","type":"string"},{"internalType":"uint8","name":"__decimals","type":"uint8"},{"internalType":"uint256","name":"_expiry","type":"uint256"}],"stateMutability":"nonpayable","type":"constructor"},{"inputs":[],"name":"InvalidShortString","type":"error"},{"inputs":[],"name":"OnlyYCFactory","type":"error"},{"inputs":[],"name":"OnlyYT","t
...
π¬ sdaftuar commented on pull request "cluster mempool: Implement changeset interface for mempool":
(https://github.com/bitcoin/bitcoin/pull/31122#discussion_r1818596465)
I think the issue is if we `quit_early`; I recall the fuzzer hitting the new `Assume()` calls via `LimitMempoolSize()` at the line just below, before I added this.
(https://github.com/bitcoin/bitcoin/pull/31122#discussion_r1818596465)
I think the issue is if we `quit_early`; I recall the fuzzer hitting the new `Assume()` calls via `LimitMempoolSize()` at the line just below, before I added this.
π¬ sdaftuar commented on pull request "cluster mempool: Implement changeset interface for mempool":
(https://github.com/bitcoin/bitcoin/pull/31122#discussion_r1818600499)
Update: I realized that at least for now, there is a requirement that any added transactions are added in a topological order, with parents coming before children -- this is so that we can add things to the mempool's multi_index at `Apply()` time without creating any missing dependencies. But removals can happen in any order and can be interleaved with the additions.
(https://github.com/bitcoin/bitcoin/pull/31122#discussion_r1818600499)
Update: I realized that at least for now, there is a requirement that any added transactions are added in a topological order, with parents coming before children -- this is so that we can add things to the mempool's multi_index at `Apply()` time without creating any missing dependencies. But removals can happen in any order and can be interleaved with the additions.
π¬ maflcko commented on pull request "bench: Remove various extraneous benchmarks":
(https://github.com/bitcoin/bitcoin/pull/31153#issuecomment-2440916567)
Hex parsing is used in mining via `DecodeHexBlk`, no?
Also, while some of the benchmarks do not exist to be optimized, but merely to give a feeling and to avoid a regression where an algorithm is swapped to go from `O(n)` to `O(n^2)`. So in a sense, they are a bit of a unit test.
In any case, updating the docs should be uncontroversial and useful going forward.
(https://github.com/bitcoin/bitcoin/pull/31153#issuecomment-2440916567)
Hex parsing is used in mining via `DecodeHexBlk`, no?
Also, while some of the benchmarks do not exist to be optimized, but merely to give a feeling and to avoid a regression where an algorithm is swapped to go from `O(n)` to `O(n^2)`. So in a sense, they are a bit of a unit test.
In any case, updating the docs should be uncontroversial and useful going forward.
π¬ maflcko commented on pull request "Introduce `g_fuzzing` global for fuzzing checks":
(https://github.com/bitcoin/bitcoin/pull/31093#issuecomment-2440960117)
review ACK 9f243cd7fa6654e3b71ba6bff82cceed547c5d53 π
<details><summary>Show signature</summary>
Signature:
```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: review ACK 9f243cd7fa66
...
(https://github.com/bitcoin/bitcoin/pull/31093#issuecomment-2440960117)
review ACK 9f243cd7fa6654e3b71ba6bff82cceed547c5d53 π
<details><summary>Show signature</summary>
Signature:
```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: review ACK 9f243cd7fa66
...
π¬ maflcko commented on pull request "bench: add support for custom data directory":
(https://github.com/bitcoin/bitcoin/pull/31000#issuecomment-2441018301)
Looks like that fixed it. However, it would be good to understand *why* it fixed it. Looking at the diff that triggered it, I fail to see how (https://github.com/bitcoin/bitcoin/pull/31000#issuecomment-2437357680)
(https://github.com/bitcoin/bitcoin/pull/31000#issuecomment-2441018301)
Looks like that fixed it. However, it would be good to understand *why* it fixed it. Looking at the diff that triggered it, I fail to see how (https://github.com/bitcoin/bitcoin/pull/31000#issuecomment-2437357680)
π¬ maflcko commented on pull request "Windows bitcoind stall debugging [NOMERGE, DRAFT]":
(https://github.com/bitcoin/bitcoin/pull/30956#issuecomment-2441027190)
Can this be closed, or is it waiting on something?
(https://github.com/bitcoin/bitcoin/pull/30956#issuecomment-2441027190)
Can this be closed, or is it waiting on something?
π¬ naumenkogs commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1818548647)
50c19dd21f6e437cd8e3d9047a89d793d097c5e5
Not sure it's worth exposing this external method if the only reason to have it is testing.
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1818548647)
50c19dd21f6e437cd8e3d9047a89d793d097c5e5
Not sure it's worth exposing this external method if the only reason to have it is testing.
π¬ naumenkogs commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1818553463)
50c19dd21f6e437cd8e3d9047a89d793d097c5e5
worth adding a comment on what would happen if there are 3 colliding transactions (probably fine but some text would be helpful) :)
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1818553463)
50c19dd21f6e437cd8e3d9047a89d793d097c5e5
worth adding a comment on what would happen if there are 3 colliding transactions (probably fine but some text would be helpful) :)
π¬ naumenkogs commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1818568903)
50c19dd21f6e437cd8e3d9047a89d793d097c5e5
could *this* be used to game collisions (for *wtxid* you have here)?
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1818568903)
50c19dd21f6e437cd8e3d9047a89d793d097c5e5
could *this* be used to game collisions (for *wtxid* you have here)?
π¬ naumenkogs commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1818583090)
392facfce1be6d0395abecc2e049ff39635bd1a5
you can technically feed it arbitrary transactions. I think the comment should say it, and then later say "we normally pass a list of parents to determine whether [... ... ...]".
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1818583090)
392facfce1be6d0395abecc2e049ff39635bd1a5
you can technically feed it arbitrary transactions. I think the comment should say it, and then later say "we normally pass a list of parents to determine whether [... ... ...]".
π¬ naumenkogs commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1818575030)
392facfce1be6d0395abecc2e049ff39635bd1a5
why do you think force relay should influence whether we reconcile or flood? The transaction is relayed in either case (the original meaning of force relay)
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1818575030)
392facfce1be6d0395abecc2e049ff39635bd1a5
why do you think force relay should influence whether we reconcile or flood? The transaction is relayed in either case (the original meaning of force relay)
π¬ naumenkogs commented on pull request "p2p: Fill reconciliation sets (Erlay) attempt 2":
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1818555798)
50c19dd21f6e437cd8e3d9047a89d793d097c5e5
what would happen if a descendant was already flooded?
(https://github.com/bitcoin/bitcoin/pull/30116#discussion_r1818555798)
50c19dd21f6e437cd8e3d9047a89d793d097c5e5
what would happen if a descendant was already flooded?