💬 hebasto commented on pull request "[26.x] backports and final changes for 26.2rc1":
(https://github.com/bitcoin/bitcoin/pull/30260#issuecomment-2158414684)
@glozow
Feel free to pick up the https://github.com/hebasto/bitcoin/commit/a586d703dce4c6fba95696ae978d9711a355d7d6 commit from the https://github.com/hebasto/bitcoin/commits/pr30260/0610 branch to pull the recent translations from the Transifex website.
Unfortunately, more translations have been vandalized (
(https://github.com/bitcoin/bitcoin/pull/30260#issuecomment-2158414684)
@glozow
Feel free to pick up the https://github.com/hebasto/bitcoin/commit/a586d703dce4c6fba95696ae978d9711a355d7d6 commit from the https://github.com/hebasto/bitcoin/commits/pr30260/0610 branch to pull the recent translations from the Transifex website.
Unfortunately, more translations have been vandalized (
💬 vasild commented on pull request "fuzz: add I2P harness":
(https://github.com/bitcoin/bitcoin/pull/30230#discussion_r1633234183)
Does this mean that the time will be frozen for the duration of the test?
From the OP:
> GetTime is called at points in i2p
Where is `GetTime()` called in i2p?
(https://github.com/bitcoin/bitcoin/pull/30230#discussion_r1633234183)
Does this mean that the time will be frozen for the duration of the test?
From the OP:
> GetTime is called at points in i2p
Where is `GetTime()` called in i2p?
💬 vasild commented on pull request "fuzz: add I2P harness":
(https://github.com/bitcoin/bitcoin/pull/30230#discussion_r1633272627)
Can be simplified:
```suggestion
CThreadInterrupt interrupt;
i2p::sam::Session session{private_key_path, Proxy{CService{}, false}, &interrupt};
```
(https://github.com/bitcoin/bitcoin/pull/30230#discussion_r1633272627)
Can be simplified:
```suggestion
CThreadInterrupt interrupt;
i2p::sam::Session session{private_key_path, Proxy{CService{}, false}, &interrupt};
```
👍 cbergqvist approved a pull request: "util: add BitSet"
(https://github.com/bitcoin/bitcoin/pull/30160#pullrequestreview-2107852785)
re-ACK 47f705b33fc1381d96c99038e2110e6fe2b2f883
Used same range-diff as instagibbs.
(https://github.com/bitcoin/bitcoin/pull/30160#pullrequestreview-2107852785)
re-ACK 47f705b33fc1381d96c99038e2110e6fe2b2f883
Used same range-diff as instagibbs.
💬 glozow commented on pull request "[26.x] backports and final changes for 26.2rc1":
(https://github.com/bitcoin/bitcoin/pull/30260#issuecomment-2158437058)
> Feel free to pick up the [hebasto@a586d70](https://github.com/hebasto/bitcoin/commit/a586d703dce4c6fba95696ae978d9711a355d7d6) commit from the https://github.com/hebasto/bitcoin/commits/pr30260/0610 branch to pull the recent translations from the Transifex website.
Thanks @hebasto! I've cherry-picked that with a minor change to the commit message
(https://github.com/bitcoin/bitcoin/pull/30260#issuecomment-2158437058)
> Feel free to pick up the [hebasto@a586d70](https://github.com/hebasto/bitcoin/commit/a586d703dce4c6fba95696ae978d9711a355d7d6) commit from the https://github.com/hebasto/bitcoin/commits/pr30260/0610 branch to pull the recent translations from the Transifex website.
Thanks @hebasto! I've cherry-picked that with a minor change to the commit message
💬 Sjors commented on pull request "assumeutxo: Add dumptxoutset height param, remove shell scripts":
(https://github.com/bitcoin/bitcoin/pull/29553#issuecomment-2158443019)
> I am still planning to do the rollback in a cleaner way but[...] It would become a refactoring follow-up that does not change the RPC interface again.
That makes sense.
(https://github.com/bitcoin/bitcoin/pull/29553#issuecomment-2158443019)
> I am still planning to do the rollback in a cleaner way but[...] It would become a refactoring follow-up that does not change the RPC interface again.
That makes sense.
💬 TheCharlatan commented on pull request "indexes: Don't wipe indexes again when continuing a prior reindex":
(https://github.com/bitcoin/bitcoin/pull/30132#issuecomment-2158458001)
https://github.com/bitcoin/bitcoin/pull/30132#issuecomment-2158381286
> I think the PR is ready to merge, so you can let me know if you want to add an assert or just merge it in its current form.
I think this is rfm.
(https://github.com/bitcoin/bitcoin/pull/30132#issuecomment-2158458001)
https://github.com/bitcoin/bitcoin/pull/30132#issuecomment-2158381286
> I think the PR is ready to merge, so you can let me know if you want to add an assert or just merge it in its current form.
I think this is rfm.
🚀 ryanofsky merged a pull request: "indexes: Don't wipe indexes again when continuing a prior reindex"
(https://github.com/bitcoin/bitcoin/pull/30132)
(https://github.com/bitcoin/bitcoin/pull/30132)
💬 hebasto commented on pull request "[26.x] backports and final changes for 26.2rc1":
(https://github.com/bitcoin/bitcoin/pull/30260#issuecomment-2158499569)
The native Windows CI fails due to a [broken Windows image](https://github.com/actions/runner-images/issues/10004).
(https://github.com/bitcoin/bitcoin/pull/30260#issuecomment-2158499569)
The native Windows CI fails due to a [broken Windows image](https://github.com/actions/runner-images/issues/10004).
✅ glozow closed a pull request: "chore: fix typos"
(https://github.com/bitcoin/bitcoin/pull/30259)
(https://github.com/bitcoin/bitcoin/pull/30259)
💬 glozow commented on pull request "chore: fix typos":
(https://github.com/bitcoin/bitcoin/pull/30259#issuecomment-2158502664)
Thanks for your interest in contributing! However, as we have hundreds of pull requests, I am closing this to focus review on the others. (See [contributing guidelines on refactoring](https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#refactoring)).
(https://github.com/bitcoin/bitcoin/pull/30259#issuecomment-2158502664)
Thanks for your interest in contributing! However, as we have hundreds of pull requests, I am closing this to focus review on the others. (See [contributing guidelines on refactoring](https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#refactoring)).
👍 stickies-v approved a pull request: "[26.x] backports and final changes for 26.2rc1"
(https://github.com/bitcoin/bitcoin/pull/30260#pullrequestreview-2107948460)
ACK 7ca424f8e651707effe1380aaf72d9ad0e97aa18
I checked that:
- backport is clean
- manpages are identical to mine
- translations are identical to mine, except for the removal of the languages mentioned in the commit message, which appears to be vandalism
- release notes are correct
(https://github.com/bitcoin/bitcoin/pull/30260#pullrequestreview-2107948460)
ACK 7ca424f8e651707effe1380aaf72d9ad0e97aa18
I checked that:
- backport is clean
- manpages are identical to mine
- translations are identical to mine, except for the removal of the languages mentioned in the commit message, which appears to be vandalism
- release notes are correct
👍 brunoerg approved a pull request: "test: Remove redundant verack check"
(https://github.com/bitcoin/bitcoin/pull/30252#pullrequestreview-2107960087)
ACK 0000276b31cea5e443a59d94a98c569293ada951
(https://github.com/bitcoin/bitcoin/pull/30252#pullrequestreview-2107960087)
ACK 0000276b31cea5e443a59d94a98c569293ada951
👍 ryanofsky approved a pull request: "Introduce Mining interface"
(https://github.com/bitcoin/bitcoin/pull/30200#pullrequestreview-2107880187)
Code review ACK febd3d1c3d41cf6083baf990fb2a3a69cade364a
(https://github.com/bitcoin/bitcoin/pull/30200#pullrequestreview-2107880187)
Code review ACK febd3d1c3d41cf6083baf990fb2a3a69cade364a
💬 ryanofsky commented on pull request "Introduce Mining interface":
(https://github.com/bitcoin/bitcoin/pull/30200#discussion_r1633297880)
In commit "rpc: getblocktemplate getTip() via Miner interface" (9c226c5c3818ef157fc50a5f6ecc534a612ac1d0)
Commit message could be updated since this now called getTipHash
(https://github.com/bitcoin/bitcoin/pull/30200#discussion_r1633297880)
In commit "rpc: getblocktemplate getTip() via Miner interface" (9c226c5c3818ef157fc50a5f6ecc534a612ac1d0)
Commit message could be updated since this now called getTipHash
💬 ryanofsky commented on pull request "Introduce Mining interface":
(https://github.com/bitcoin/bitcoin/pull/30200#discussion_r1633296276)
In commit "rpc: call TestBlockValidity via miner interface" (8589e6c44726d6b5760095d72c37934e4be3315c)
Why is the `testBlockValidity` check being moved before the block.hashPrevBlock != tip check? The previous ordering seemed more straightforward and consistent with "TestBlockValidity only supports blocks built on the current Tip" comment
(https://github.com/bitcoin/bitcoin/pull/30200#discussion_r1633296276)
In commit "rpc: call TestBlockValidity via miner interface" (8589e6c44726d6b5760095d72c37934e4be3315c)
Why is the `testBlockValidity` check being moved before the block.hashPrevBlock != tip check? The previous ordering seemed more straightforward and consistent with "TestBlockValidity only supports blocks built on the current Tip" comment
💬 ryanofsky commented on pull request "Introduce Mining interface":
(https://github.com/bitcoin/bitcoin/pull/30200#discussion_r1633318785)
In commit "rpc: call CreateNewBlock via miner interface" (f375fb9a0dff7ccead80626f4aee853db7799e6f)
It might be good to keep this ApplyArgsManOptions function for more consistency with other options parsing code. The node/interfaces.cpp createNewBlock function could call ApplyArgsManOptions instead of parsing options itself. The current approach also seems ok though if you prefer it.
(https://github.com/bitcoin/bitcoin/pull/30200#discussion_r1633318785)
In commit "rpc: call CreateNewBlock via miner interface" (f375fb9a0dff7ccead80626f4aee853db7799e6f)
It might be good to keep this ApplyArgsManOptions function for more consistency with other options parsing code. The node/interfaces.cpp createNewBlock function could call ApplyArgsManOptions instead of parsing options itself. The current approach also seems ok though if you prefer it.
💬 ryanofsky commented on pull request "Introduce Mining interface":
(https://github.com/bitcoin/bitcoin/pull/30200#discussion_r1633363110)
In commit "rpc: call CreateNewBlock via miner interface" (f375fb9a0dff7ccead80626f4aee853db7799e6f)
I'm not clear how this implementation lines up with the commit description which says "the pool communicates how many extra bytes it needs for its own outputs (payouts, extra commitments, etc). This needs to be substracted from what the user set as -blockmaxweight."
Is the commit description just saying that this implementation is not complete and it will need to be extended in the future? W
...
(https://github.com/bitcoin/bitcoin/pull/30200#discussion_r1633363110)
In commit "rpc: call CreateNewBlock via miner interface" (f375fb9a0dff7ccead80626f4aee853db7799e6f)
I'm not clear how this implementation lines up with the commit description which says "the pool communicates how many extra bytes it needs for its own outputs (payouts, extra commitments, etc). This needs to be substracted from what the user set as -blockmaxweight."
Is the commit description just saying that this implementation is not complete and it will need to be extended in the future? W
...
👍 ryanofsky approved a pull request: "validation: improve performance of CheckBlockIndex"
(https://github.com/bitcoin/bitcoin/pull/28339#pullrequestreview-2108028986)
Code review ACK 5bc2077e8f592442b089affdf0b5795fbc053bb8. Just added suggested assert and simplification since last review
(https://github.com/bitcoin/bitcoin/pull/28339#pullrequestreview-2108028986)
Code review ACK 5bc2077e8f592442b089affdf0b5795fbc053bb8. Just added suggested assert and simplification since last review
💬 Sjors commented on pull request "Introduce Mining interface":
(https://github.com/bitcoin/bitcoin/pull/30200#discussion_r1633391557)
Yes, this is a reference to the future change in sv2. I'll see if I can reword it.
(https://github.com/bitcoin/bitcoin/pull/30200#discussion_r1633391557)
Yes, this is a reference to the future change in sv2. I'll see if I can reword it.