π¬ danielabrozzoni commented on pull request "p2p: rename GetAddresses -> GetAddressesUncached":
(https://github.com/bitcoin/bitcoin/pull/32994#discussion_r2219096854)
I also don't love the `GetAddresses`/`GetAddressesUncached` naming either, but I agree with stickies that it's important that the unsafe version stands out.
What if we instead renamed them to `GetAddressesCached` / `GetAddressesUncached`? That way, thereβs no ambiguity about what each function does, and the naming feels a bit more consistent.
(https://github.com/bitcoin/bitcoin/pull/32994#discussion_r2219096854)
I also don't love the `GetAddresses`/`GetAddressesUncached` naming either, but I agree with stickies that it's important that the unsafe version stands out.
What if we instead renamed them to `GetAddressesCached` / `GetAddressesUncached`? That way, thereβs no ambiguity about what each function does, and the naming feels a bit more consistent.
π¬ Eunovo commented on pull request "Silent Payments: Receiving":
(https://github.com/bitcoin/bitcoin/pull/32966#discussion_r2219135907)
> This makes sense. Is it documented anywhere so we don't forget by the next review round? :-)
I added a separate commit that disables fast rescan and I added an explanation to the commit message. Will push soon.
(https://github.com/bitcoin/bitcoin/pull/32966#discussion_r2219135907)
> This makes sense. Is it documented anywhere so we don't forget by the next review round? :-)
I added a separate commit that disables fast rescan and I added an explanation to the commit message. Will push soon.
π¬ m3dwards commented on pull request "Migrate CI to hosted Cirrus Runners":
(https://github.com/bitcoin/bitcoin/pull/32989#discussion_r2219202190)
Done
(https://github.com/bitcoin/bitcoin/pull/32989#discussion_r2219202190)
Done
π¬ maflcko commented on pull request "multiprocess: Add capnp wrapper for Chain interface":
(https://github.com/bitcoin/bitcoin/pull/29409#discussion_r2219238228)
implementat -> implement [spelling error in comment ββ¦able to implementat a ReadFieldβ¦β]
(https://github.com/bitcoin/bitcoin/pull/29409#discussion_r2219238228)
implementat -> implement [spelling error in comment ββ¦able to implementat a ReadFieldβ¦β]
π¬ darosior commented on pull request "script: return verification flag responsible for error upon validation failure":
(https://github.com/bitcoin/bitcoin/pull/33012#issuecomment-3096821342)
The behaviour already exists in a limited fashion. Even without this change, an attacker can trivially create such a transaction (as discussed in OP). Here is for instance a `p2p_invalid_tx.py` case which passes both with or without this PR:
<details>
<summary> Click to see patch </summary>
```diff
diff --git a/test/functional/data/invalid_txs.py b/test/functional/data/invalid_txs.py
index 36b274efc27..46bb2bba367 100644
--- a/test/functional/data/invalid_txs.py
+++ b/test/functiona
...
(https://github.com/bitcoin/bitcoin/pull/33012#issuecomment-3096821342)
The behaviour already exists in a limited fashion. Even without this change, an attacker can trivially create such a transaction (as discussed in OP). Here is for instance a `p2p_invalid_tx.py` case which passes both with or without this PR:
<details>
<summary> Click to see patch </summary>
```diff
diff --git a/test/functional/data/invalid_txs.py b/test/functional/data/invalid_txs.py
index 36b274efc27..46bb2bba367 100644
--- a/test/functional/data/invalid_txs.py
+++ b/test/functiona
...
π¬ marcofleon commented on pull request "refactor: GenTxid type safety followups":
(https://github.com/bitcoin/bitcoin/pull/33005#discussion_r2219264339)
Fair, it does feel cleaner using GenTxid and avoiding the branch in `ProcessGetData`. I chose to templatize in response to this [comment](https://github.com/bitcoin/bitcoin/pull/32631#pullrequestreview-2877628308) from the original PR. The templating takes care of everything at compile-time, which is nice. But `m_most_recent_block_txs` holds GenTxids, so in the templated version we're creating a temporary GenTxid anyway for that map lookup vs creating a GenTxid once upfront.
I think it's larg
...
(https://github.com/bitcoin/bitcoin/pull/33005#discussion_r2219264339)
Fair, it does feel cleaner using GenTxid and avoiding the branch in `ProcessGetData`. I chose to templatize in response to this [comment](https://github.com/bitcoin/bitcoin/pull/32631#pullrequestreview-2877628308) from the original PR. The templating takes care of everything at compile-time, which is nice. But `m_most_recent_block_txs` holds GenTxids, so in the templated version we're creating a temporary GenTxid anyway for that map lookup vs creating a GenTxid once upfront.
I think it's larg
...
π¬ danielabrozzoni commented on pull request "p2p: rename GetAddresses -> GetAddressesUncached":
(https://github.com/bitcoin/bitcoin/pull/32994#issuecomment-3096922403)
In my last push (df65b527f00cbf9fd6675bf22bb1285bfce24f66):
- split changes into two commits (one for rename, one for docs)
(https://github.com/bitcoin/bitcoin/pull/32994#issuecomment-3096922403)
In my last push (df65b527f00cbf9fd6675bf22bb1285bfce24f66):
- split changes into two commits (one for rename, one for docs)
π¬ totdking commented on issue "A missing import to the src/chainparamsbase.h":
(https://github.com/bitcoin/bitcoin/issues/33019#issuecomment-3096966958)
I added the import and it worked :) , thanks man π
(https://github.com/bitcoin/bitcoin/issues/33019#issuecomment-3096966958)
I added the import and it worked :) , thanks man π
π¬ maflcko commented on pull request "docs: adds correct updated documentation links":
(https://github.com/bitcoin/bitcoin/pull/32699#issuecomment-3096984862)
No, you haven't replied/addressed the feedback from June: https://github.com/bitcoin/bitcoin/pull/32699#discussion_r2150261914
(https://github.com/bitcoin/bitcoin/pull/32699#issuecomment-3096984862)
No, you haven't replied/addressed the feedback from June: https://github.com/bitcoin/bitcoin/pull/32699#discussion_r2150261914
β
pinheadmz closed an issue: "b-msghand invoked oom-killer in Debug build: Master (v28.99) crashing during IBD"
(https://github.com/bitcoin/bitcoin/issues/31561)
(https://github.com/bitcoin/bitcoin/issues/31561)
π¬ pinheadmz commented on issue "b-msghand invoked oom-killer in Debug build: Master (v28.99) crashing during IBD":
(https://github.com/bitcoin/bitcoin/issues/31561#issuecomment-3096984988)
Closing this now, `Bitcoin Core daemon version v29.99.0-c43cc48aaaaa (debug build)` synced succesfully on a debian VM 8GB memory and no swap
(https://github.com/bitcoin/bitcoin/issues/31561#issuecomment-3096984988)
Closing this now, `Bitcoin Core daemon version v29.99.0-c43cc48aaaaa (debug build)` synced succesfully on a debian VM 8GB memory and no swap
π¬ sr-gi commented on pull request "Fix tiebreak when loading blocks from disk (and add tests for comparing chain ties)":
(https://github.com/bitcoin/bitcoin/pull/29640#discussion_r2219412158)
Rebased and fixed
(https://github.com/bitcoin/bitcoin/pull/29640#discussion_r2219412158)
Rebased and fixed
π¬ darosior commented on pull request "script: return verification flag responsible for error upon validation failure":
(https://github.com/bitcoin/bitcoin/pull/33012#issuecomment-3097046877)
Rebased to fix a silent merge conflict that was making the "test each commit" CI job fail.
(https://github.com/bitcoin/bitcoin/pull/33012#issuecomment-3097046877)
Rebased to fix a silent merge conflict that was making the "test each commit" CI job fail.
π¬ sr-gi commented on pull request "Fix tiebreak when loading blocks from disk (and add tests for comparing chain ties)":
(https://github.com/bitcoin/bitcoin/pull/29640#issuecomment-3097049530)
Rebased to switch from the old (removed) `CBlock::hash` property to the recently introduced `hash_int` and `hash_hex`
(https://github.com/bitcoin/bitcoin/pull/29640#issuecomment-3097049530)
Rebased to switch from the old (removed) `CBlock::hash` property to the recently introduced `hash_int` and `hash_hex`
π¬ glozow commented on pull request "mempool: Avoid needless vtx iteration during IBD":
(https://github.com/bitcoin/bitcoin/pull/32827#issuecomment-3097123255)
ACK 249889bee6b88eb9814eb969e6fb108f86a4bf98
(https://github.com/bitcoin/bitcoin/pull/32827#issuecomment-3097123255)
ACK 249889bee6b88eb9814eb969e6fb108f86a4bf98
π¬ glozow commented on pull request "[29.x] Backport #32521":
(https://github.com/bitcoin/bitcoin/pull/33013#issuecomment-3097142325)
@darosior could you make the release notes updates as well please? Thanks
(https://github.com/bitcoin/bitcoin/pull/33013#issuecomment-3097142325)
@darosior could you make the release notes updates as well please? Thanks
π glozow merged a pull request: "mempool: Avoid needless vtx iteration during IBD"
(https://github.com/bitcoin/bitcoin/pull/32827)
(https://github.com/bitcoin/bitcoin/pull/32827)
π¬ sipa commented on pull request "script: return verification flag responsible for error upon validation failure":
(https://github.com/bitcoin/bitcoin/pull/33012#discussion_r2219485859)
This has no effect. If `err_flag` is already zero, then `err_flah &= ...;` will not change that.
(https://github.com/bitcoin/bitcoin/pull/33012#discussion_r2219485859)
This has no effect. If `err_flag` is already zero, then `err_flah &= ...;` will not change that.
π¬ darosior commented on pull request "script: return verification flag responsible for error upon validation failure":
(https://github.com/bitcoin/bitcoin/pull/33012#discussion_r2219502307)
Oops, thanks. Of course i meant `|=` here.
(https://github.com/bitcoin/bitcoin/pull/33012#discussion_r2219502307)
Oops, thanks. Of course i meant `|=` here.
π¬ darosior commented on pull request "script: return verification flag responsible for error upon validation failure":
(https://github.com/bitcoin/bitcoin/pull/33012#discussion_r2219503216)
There is a couple other occurrences where i must have made the same mistake.
(https://github.com/bitcoin/bitcoin/pull/33012#discussion_r2219503216)
There is a couple other occurrences where i must have made the same mistake.