π fanquake merged a pull request: "[27.1] Finalize"
(https://github.com/bitcoin/bitcoin/pull/30222)
(https://github.com/bitcoin/bitcoin/pull/30222)
π¬ ismaelsadeeq commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1634885258)
nit:
```suggestion
// 1 parent paying 199sat, 1 child paying 1300sat.
```
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1634885258)
nit:
```suggestion
// 1 parent paying 199sat, 1 child paying 1300sat.
```
π¬ ryanofsky commented on pull request "Introduce Mining interface":
(https://github.com/bitcoin/bitcoin/pull/30200#issuecomment-2160764070)
> > Additionally the template provider needs to wait, to see if there's a new tip, with g_best_block_cv.wait_until. How would I go about moving that into this interface?
Maybe the simplest way to do it would be to have a waitTipChanged() method that just blocks waiting for a new tip, and would be interrupted when the node shuts down or the mining interface is destroyed. If needed, the waiting interface could have more features and accept a timeout or return an object with wait() and cancel()
...
(https://github.com/bitcoin/bitcoin/pull/30200#issuecomment-2160764070)
> > Additionally the template provider needs to wait, to see if there's a new tip, with g_best_block_cv.wait_until. How would I go about moving that into this interface?
Maybe the simplest way to do it would be to have a waitTipChanged() method that just blocks waiting for a new tip, and would be interrupted when the node shuts down or the mining interface is destroyed. If needed, the waiting interface could have more features and accept a timeout or return an object with wait() and cancel()
...
π fanquake approved a pull request: "[26.x] backports and final changes for 26.2rc1"
(https://github.com/bitcoin/bitcoin/pull/30260#pullrequestreview-2110469350)
ACK 7ca424f8e651707effe1380aaf72d9ad0e97aa18
(https://github.com/bitcoin/bitcoin/pull/30260#pullrequestreview-2110469350)
ACK 7ca424f8e651707effe1380aaf72d9ad0e97aa18
π theuni approved a pull request: "depends: remove `FORCE_USE_SYSTEM_CLANG`"
(https://github.com/bitcoin/bitcoin/pull/30201#pullrequestreview-2110480884)
Tested on Ubuntu 22.04 with downloaded llvm 18 binaries in PATH.
ACK 7cbfd7a7ce45ac68d6041f42f468862f5c193d8c
(https://github.com/bitcoin/bitcoin/pull/30201#pullrequestreview-2110480884)
Tested on Ubuntu 22.04 with downloaded llvm 18 binaries in PATH.
ACK 7cbfd7a7ce45ac68d6041f42f468862f5c193d8c
π¬ instagibbs commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1634923804)
done, and found one more place with out of date comment
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1634923804)
done, and found one more place with out of date comment
π¬ instagibbs commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1634923975)
done
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1634923975)
done
π¬ ryanofsky commented on pull request "test: Assumeutxo: import snapshot in a node with a divergent chain":
(https://github.com/bitcoin/bitcoin/pull/29996#issuecomment-2160819048)
> This PR adds tests to cover two scenarios of loading a snapshot when the current chain tip is:
>
> * Not an ancestor of the snapshot block but has less work
>
> * Not an ancestor or a descendant of the snapshot block and has more work
>
> In the second scenario, the snapshot block does not belong to the most-work chain anymore so I believe it covers this scenario too: `TODO: Valid snapshot file and snapshot block, but the block is not on the most-work chain`. Therefore I delet
...
(https://github.com/bitcoin/bitcoin/pull/29996#issuecomment-2160819048)
> This PR adds tests to cover two scenarios of loading a snapshot when the current chain tip is:
>
> * Not an ancestor of the snapshot block but has less work
>
> * Not an ancestor or a descendant of the snapshot block and has more work
>
> In the second scenario, the snapshot block does not belong to the most-work chain anymore so I believe it covers this scenario too: `TODO: Valid snapshot file and snapshot block, but the block is not on the most-work chain`. Therefore I delet
...
π€ theuni reviewed a pull request: "build: Bump clang minimum supported version to 16"
(https://github.com/bitcoin/bitcoin/pull/30263#pullrequestreview-2110536012)
Concept ACK.
FWIW on Ubuntu 20.04 I'm using pre-compiled binaries from https://github.com/llvm/llvm-project/releases/tag/llvmorg-18.1.4 with no issues.
(https://github.com/bitcoin/bitcoin/pull/30263#pullrequestreview-2110536012)
Concept ACK.
FWIW on Ubuntu 20.04 I'm using pre-compiled binaries from https://github.com/llvm/llvm-project/releases/tag/llvmorg-18.1.4 with no issues.
π tdb3 approved a pull request: "test: add coverage for errors for `combinerawtransaction`"
(https://github.com/bitcoin/bitcoin/pull/30264#pullrequestreview-2110589147)
ACK ab98e6fd03970d6b5a593674c84e762a47b90ea6
Thanks for adding coverage. The new asserts look like they fit in nicely with the existing test code. Ran `rpc_transaction` locally (passed).
(https://github.com/bitcoin/bitcoin/pull/30264#pullrequestreview-2110589147)
ACK ab98e6fd03970d6b5a593674c84e762a47b90ea6
Thanks for adding coverage. The new asserts look like they fit in nicely with the existing test code. Ran `rpc_transaction` locally (passed).
π¬ theuni commented on pull request "doc: add release note for 29091 and 29165":
(https://github.com/bitcoin/bitcoin/pull/30261#issuecomment-2160864707)
Wait for https://github.com/bitcoin/bitcoin/pull/30263 ?
(https://github.com/bitcoin/bitcoin/pull/30261#issuecomment-2160864707)
Wait for https://github.com/bitcoin/bitcoin/pull/30263 ?
π fanquake merged a pull request: "test: add coverage for errors for `combinerawtransaction`"
(https://github.com/bitcoin/bitcoin/pull/30264)
(https://github.com/bitcoin/bitcoin/pull/30264)
π¬ brunoerg commented on pull request "fuzz: add I2P harness":
(https://github.com/bitcoin/bitcoin/pull/30230#discussion_r1635006887)
> Where is GetTime() called in i2p?
It's used in `Sock` (e.g. `RecvUntilTerminator`).
(https://github.com/bitcoin/bitcoin/pull/30230#discussion_r1635006887)
> Where is GetTime() called in i2p?
It's used in `Sock` (e.g. `RecvUntilTerminator`).
π¬ theuni commented on pull request "refactor: reserve memory allocation for transaction outputs":
(https://github.com/bitcoin/bitcoin/pull/30093#issuecomment-2160956259)
Changes look good. The bench is not really useful though, because it's testing things that aren't in our code.
I believe @josibake was asking for a bench that demonstrates a before/after of `CreateTransactionInternal`. I'm guessing that's not really feasible though, so I think it's enough to use your bench numbers without actually committing it.
(https://github.com/bitcoin/bitcoin/pull/30093#issuecomment-2160956259)
Changes look good. The bench is not really useful though, because it's testing things that aren't in our code.
I believe @josibake was asking for a bench that demonstrates a before/after of `CreateTransactionInternal`. I'm guessing that's not really feasible though, so I think it's enough to use your bench numbers without actually committing it.
π¬ paplorinc commented on pull request "refactor: reserve memory allocation for transaction outputs":
(https://github.com/bitcoin/bitcoin/pull/30093#issuecomment-2160989550)
Moved the benchmark out to https://gist.github.com/paplorinc/812007eef71d5285be0654375ea3e03e
(https://github.com/bitcoin/bitcoin/pull/30093#issuecomment-2160989550)
Moved the benchmark out to https://gist.github.com/paplorinc/812007eef71d5285be0654375ea3e03e
π¬ ajtowns commented on pull request "Testnet4 including PoW difficulty adjustment fix":
(https://github.com/bitcoin/bitcoin/pull/29775#issuecomment-2160992009)
> I think that could reduce the difficulty by up to a factor of 16 (if you are willing to wait up to eight weeks), but I donβt see how someone needing to manually intervene and most likely still needing an ASIC mitigates the potential liveness issue here.
If someone's attacking testnet with 4x more hashpower than the rest of the network (ie, 80% hash rate), can't they just do a 50% attack and mine empty blocks? If you're trying to run a chain with minority hashpower you need to do something l
...
(https://github.com/bitcoin/bitcoin/pull/29775#issuecomment-2160992009)
> I think that could reduce the difficulty by up to a factor of 16 (if you are willing to wait up to eight weeks), but I donβt see how someone needing to manually intervene and most likely still needing an ASIC mitigates the potential liveness issue here.
If someone's attacking testnet with 4x more hashpower than the rest of the network (ie, 80% hash rate), can't they just do a 50% attack and mine empty blocks? If you're trying to run a chain with minority hashpower you need to do something l
...
π¬ ryanofsky commented on pull request "test: Assumeutxo: import snapshot in a node with a divergent chain":
(https://github.com/bitcoin/bitcoin/pull/29996#discussion_r1635022608)
re: https://github.com/bitcoin/bitcoin/pull/29996/files#r1633915935
> my proposed fix would be something like [mzumsande@edb2b69](https://github.com/mzumsande/bitcoin/commit/edb2b69a16889552ddd40b71a491e7722d9b4e12) (feel free to cherry-pick/ adjust as you like).
Nice find! Would suggest opening a separate PR so it is easier to understand the problem and fix. And maybe it is possible to come up with a simpler test for this problem specifically, like by adding an assert in FindNextBlocks()
...
(https://github.com/bitcoin/bitcoin/pull/29996#discussion_r1635022608)
re: https://github.com/bitcoin/bitcoin/pull/29996/files#r1633915935
> my proposed fix would be something like [mzumsande@edb2b69](https://github.com/mzumsande/bitcoin/commit/edb2b69a16889552ddd40b71a491e7722d9b4e12) (feel free to cherry-pick/ adjust as you like).
Nice find! Would suggest opening a separate PR so it is easier to understand the problem and fix. And maybe it is possible to come up with a simpler test for this problem specifically, like by adding an assert in FindNextBlocks()
...
π¬ theuni commented on pull request "refactor: reserve memory allocation for transaction outputs":
(https://github.com/bitcoin/bitcoin/pull/30093#discussion_r1635062430)
One last nit: because it's not modified after creation, `const auto& txout`
(https://github.com/bitcoin/bitcoin/pull/30093#discussion_r1635062430)
One last nit: because it's not modified after creation, `const auto& txout`
π¬ paplorinc commented on pull request "refactor: reserve memory allocation for transaction outputs":
(https://github.com/bitcoin/bitcoin/pull/30093#discussion_r1635065616)
Done, thanks
(https://github.com/bitcoin/bitcoin/pull/30093#discussion_r1635065616)
Done, thanks
π¬ ryanofsky commented on pull request "test: Assumeutxo: import snapshot in a node with a divergent chain":
(https://github.com/bitcoin/bitcoin/pull/29996#discussion_r1635083945)
re: https://github.com/bitcoin/bitcoin/pull/29996#discussion_r1634607244
> To clarify the meaning of the Todo:
>
> > Interesting starting states could be loading a snapshot when the current chain tip is:
> >
> > * TODO: Not an ancestor of the snapshot block but has less work
>
> Particularly "but has less work" could mean A) less work than the tip of the chain that includes the snapshot or B) less work than the snapshot block itself. I believe A is the more interesting and intended s
...
(https://github.com/bitcoin/bitcoin/pull/29996#discussion_r1635083945)
re: https://github.com/bitcoin/bitcoin/pull/29996#discussion_r1634607244
> To clarify the meaning of the Todo:
>
> > Interesting starting states could be loading a snapshot when the current chain tip is:
> >
> > * TODO: Not an ancestor of the snapshot block but has less work
>
> Particularly "but has less work" could mean A) less work than the tip of the chain that includes the snapshot or B) less work than the snapshot block itself. I believe A is the more interesting and intended s
...