👍 brunoerg approved a pull request: "rest: Reject negative outpoint index early in getutxos parsing"
(https://github.com/bitcoin/bitcoin/pull/30444#pullrequestreview-2186107158)
reACK fac932bf93d9bd8cb7bef93f04ecf48ea2ccf1d1
(https://github.com/bitcoin/bitcoin/pull/30444#pullrequestreview-2186107158)
reACK fac932bf93d9bd8cb7bef93f04ecf48ea2ccf1d1
💬 luisschwab commented on issue "`scantxoutset` should output the block hash instead of block height":
(https://github.com/bitcoin/bitcoin/issues/30478#issuecomment-2236816129)
Yes, I took the example out of the best block at the time. Indeed, it may be best to add the extra field instead.
(https://github.com/bitcoin/bitcoin/issues/30478#issuecomment-2236816129)
Yes, I took the example out of the best block at the time. Indeed, it may be best to add the extra field instead.
💬 instagibbs commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1683029030)
Both benchmarks are doing LIMO with ancestor sort, right? The only difference is in cluster shape.
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1683029030)
Both benchmarks are doing LIMO with ancestor sort, right? The only difference is in cluster shape.
💬 sipa commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1683032146)
The "ANC" one is testing a scenario that is a worst-case for ancestor sort (but is trivial for the LIMO aspect), the "LIMO" one is testing a scenario that is a worst-case for LIMO (but is trivial for the ancestor sorting aspect).
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1683032146)
The "ANC" one is testing a scenario that is a worst-case for ancestor sort (but is trivial for the LIMO aspect), the "LIMO" one is testing a scenario that is a worst-case for LIMO (but is trivial for the ancestor sorting aspect).
💬 instagibbs commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1683034976)
ah, parsing the name wrong basically, thanks
(https://github.com/bitcoin/bitcoin/pull/30126#discussion_r1683034976)
ah, parsing the name wrong basically, thanks
💬 maflcko commented on issue "`scantxoutset` should output the block hash instead of block height":
(https://github.com/bitcoin/bitcoin/issues/30478#issuecomment-2236846918)
> (and perhaps mark height aa deprecated?).
Not sure. It already exists and is convenient. If a user wants to shoot themselves in the foot, they can continue to do so by turning the block hash into a block height themselves. So I don't think there is a strong benefit in removing it?
(https://github.com/bitcoin/bitcoin/issues/30478#issuecomment-2236846918)
> (and perhaps mark height aa deprecated?).
Not sure. It already exists and is convenient. If a user wants to shoot themselves in the foot, they can continue to do so by turning the block hash into a block height themselves. So I don't think there is a strong benefit in removing it?
💬 instagibbs commented on pull request "cluster mempool: cluster linearization algorithm":
(https://github.com/bitcoin/bitcoin/pull/30126#issuecomment-2236852405)
ACK https://github.com/bitcoin/bitcoin/pull/30126/commits/6160ccf9b4327649e9bb0293fba630a10b3befc3
reviewed via `git range-diff master 2003bb8a279c8891e55bab190ca36f0c6c8697ea 6160ccf9b4327649e9bb0293fba630a10b3befc3`
I didn't validate that the BFS optimisation for search improved results, but it at least doesn't seem to hurt benchmark performance.
(https://github.com/bitcoin/bitcoin/pull/30126#issuecomment-2236852405)
ACK https://github.com/bitcoin/bitcoin/pull/30126/commits/6160ccf9b4327649e9bb0293fba630a10b3befc3
reviewed via `git range-diff master 2003bb8a279c8891e55bab190ca36f0c6c8697ea 6160ccf9b4327649e9bb0293fba630a10b3befc3`
I didn't validate that the BFS optimisation for search improved results, but it at least doesn't seem to hurt benchmark performance.
💬 instagibbs commented on pull request "Fee Estimation: change `estimatesmartfee` default mode to `economical`":
(https://github.com/bitcoin/bitcoin/pull/30275#issuecomment-2236871071)
concept ACK
could we return a new field that says the mode, then add a test for that to prevent regressions?
(https://github.com/bitcoin/bitcoin/pull/30275#issuecomment-2236871071)
concept ACK
could we return a new field that says the mode, then add a test for that to prevent regressions?
💬 theuni commented on pull request "build: Introduce CMake-based buid system":
(https://github.com/bitcoin/bitcoin/pull/30454#issuecomment-2236884983)
## Our plan for the next few weeks:
This PR has already pointed out some very helpful high-level issues.
Because we don't want to create too much noise (yet) in this PR, we're going to maintain the staging workflow for another ~ two weeks. This means discussing and merging into @hebasto's repo, then occasionally syncing with the upstream PR. More specifically:
### Goals for week 1:
- Upstream (and hopefully get merged) all outstanding outstanding issues/prerequisites pointed out by r
...
(https://github.com/bitcoin/bitcoin/pull/30454#issuecomment-2236884983)
## Our plan for the next few weeks:
This PR has already pointed out some very helpful high-level issues.
Because we don't want to create too much noise (yet) in this PR, we're going to maintain the staging workflow for another ~ two weeks. This means discussing and merging into @hebasto's repo, then occasionally syncing with the upstream PR. More specifically:
### Goals for week 1:
- Upstream (and hopefully get merged) all outstanding outstanding issues/prerequisites pointed out by r
...
💬 maflcko commented on issue "Feature Request: Broadcast Pool":
(https://github.com/bitcoin/bitcoin/issues/30471#issuecomment-2236887064)
> > Yes, but that should be harmless, as a confirmed transaction in the chain can be retrieved from the chain. The same problem would exist in a broadcast pool, because any transaction in it is not guaranteed to be confirmed, and an earlier (or later) replacement could be confirmed, or none at all.
>
> I don't agree - it could lead to a situation where a user is fee bumping a transaction unnecessarily. Certainly there are no guarantees on confirmation, but where a transaction is discarded loc
...
(https://github.com/bitcoin/bitcoin/issues/30471#issuecomment-2236887064)
> > Yes, but that should be harmless, as a confirmed transaction in the chain can be retrieved from the chain. The same problem would exist in a broadcast pool, because any transaction in it is not guaranteed to be confirmed, and an earlier (or later) replacement could be confirmed, or none at all.
>
> I don't agree - it could lead to a situation where a user is fee bumping a transaction unnecessarily. Certainly there are no guarantees on confirmation, but where a transaction is discarded loc
...
💬 andrewtoth commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1683074856)
It was just easier to split it up into 2 commits this way.
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1683074856)
It was just easier to split it up into 2 commits this way.
💬 andrewtoth commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1683076133)
Hmm this was added by the cherry-picked commit from @sipa. Can we leave it for a follow-up?
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1683076133)
Hmm this was added by the cherry-picked commit from @sipa. Can we leave it for a follow-up?
💬 ismaelsadeeq commented on pull request "Fee Estimation: change `estimatesmartfee` default mode to `economical`":
(https://github.com/bitcoin/bitcoin/pull/30275#issuecomment-2236898769)
> Could we return a new field that indicates the mode?
Yes, I plan on addressing this https://github.com/bitcoin/bitcoin/commit/2ee3d774815f5e1840cb37d2a92007f19c217e4c, in a follow-up PR: as suggested here https://github.com/bitcoin/bitcoin/pull/30275#discussion_r1682618237 along with some documentation updates proposed in #18217
I will indicate the mode as well in that follow-up
> then add a test for that to prevent regressions?
Could you please clarify about the test what specific
...
(https://github.com/bitcoin/bitcoin/pull/30275#issuecomment-2236898769)
> Could we return a new field that indicates the mode?
Yes, I plan on addressing this https://github.com/bitcoin/bitcoin/commit/2ee3d774815f5e1840cb37d2a92007f19c217e4c, in a follow-up PR: as suggested here https://github.com/bitcoin/bitcoin/pull/30275#discussion_r1682618237 along with some documentation updates proposed in #18217
I will indicate the mode as well in that follow-up
> then add a test for that to prevent regressions?
Could you please clarify about the test what specific
...
💬 andrewtoth commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1683090988)
Fixed.
(https://github.com/bitcoin/bitcoin/pull/28280#discussion_r1683090988)
Fixed.
📝 fanquake converted_to_draft a pull request: "build: Introduce CMake-based buid system"
(https://github.com/bitcoin/bitcoin/pull/30454)
This PR introduces a new CMake-based build system, which is a drop-in replacement for the current Autotools-based build system.
As discussed during the recent CoreDev meetup in April, the switch from Autotools to CMake is intended to happen as soon as possible after branching 28.x off, which means that 29.0 will be built using CMake.
This PR branch is essentially the [staging branch](https://github.com/hebasto/bitcoin/tree/cmake-staging), with every change reviewed and tested by a group of
...
(https://github.com/bitcoin/bitcoin/pull/30454)
This PR introduces a new CMake-based build system, which is a drop-in replacement for the current Autotools-based build system.
As discussed during the recent CoreDev meetup in April, the switch from Autotools to CMake is intended to happen as soon as possible after branching 28.x off, which means that 29.0 will be built using CMake.
This PR branch is essentially the [staging branch](https://github.com/hebasto/bitcoin/tree/cmake-staging), with every change reviewed and tested by a group of
...
💬 glozow commented on pull request "test: Non-Shy version sender":
(https://github.com/bitcoin/bitcoin/pull/30453#issuecomment-2236923242)
ACK fa0e227fd0c
(https://github.com/bitcoin/bitcoin/pull/30453#issuecomment-2236923242)
ACK fa0e227fd0c
💬 glozow commented on pull request "test: Non-Shy version sender":
(https://github.com/bitcoin/bitcoin/pull/30453#issuecomment-2236928375)
Oops. ACK faed5d3870f
(https://github.com/bitcoin/bitcoin/pull/30453#issuecomment-2236928375)
Oops. ACK faed5d3870f
🤔 glozow reviewed a pull request: "test: Non-Shy version sender"
(https://github.com/bitcoin/bitcoin/pull/30453#pullrequestreview-2186272759)
ACK faed5d3870f
(https://github.com/bitcoin/bitcoin/pull/30453#pullrequestreview-2186272759)
ACK faed5d3870f
💬 fanquake commented on pull request "depends: build FreeType with CMake":
(https://github.com/bitcoin/bitcoin/pull/29880#issuecomment-2236937905)
Guix Build (x86_64, aarch64):
```bash
bd6c916cc5fdd322bf9e7e3cecbe45a68551c0135d63b35709b32fccde289834 guix-build-ff4f3deb7b8a/output/aarch64-linux-gnu/SHA256SUMS.part
12c161c6b94ccb15d420fe27f7a812c95247a2614db5f8c6e666e04b5ef0a7d3 guix-build-ff4f3deb7b8a/output/aarch64-linux-gnu/bitcoin-ff4f3deb7b8a-aarch64-linux-gnu-debug.tar.gz
99c6d258715f38c7b56eefd0b08e80552012e8903f26d28225b690f432839545 guix-build-ff4f3deb7b8a/output/aarch64-linux-gnu/bitcoin-ff4f3deb7b8a-aarch64-linux-gnu.tar.gz
...
(https://github.com/bitcoin/bitcoin/pull/29880#issuecomment-2236937905)
Guix Build (x86_64, aarch64):
```bash
bd6c916cc5fdd322bf9e7e3cecbe45a68551c0135d63b35709b32fccde289834 guix-build-ff4f3deb7b8a/output/aarch64-linux-gnu/SHA256SUMS.part
12c161c6b94ccb15d420fe27f7a812c95247a2614db5f8c6e666e04b5ef0a7d3 guix-build-ff4f3deb7b8a/output/aarch64-linux-gnu/bitcoin-ff4f3deb7b8a-aarch64-linux-gnu-debug.tar.gz
99c6d258715f38c7b56eefd0b08e80552012e8903f26d28225b690f432839545 guix-build-ff4f3deb7b8a/output/aarch64-linux-gnu/bitcoin-ff4f3deb7b8a-aarch64-linux-gnu.tar.gz
...
💬 Sjors commented on pull request "Introduce waitFeesChanged() mining interface":
(https://github.com/bitcoin/bitcoin/pull/30443#issuecomment-2236950771)
@sdaftuar @sipa I made a branch that combines #28676 with #29432: https://github.com/Sjors/bitcoin/tree/sv2-cluster
The main commit is https://github.com/bitcoin/bitcoin/commit/f646977ddac0622889d484683ae59c262d50d8dc. It slightly changes the `waitFeesChanged()` interface. The implementation now frequently creates a new template.
The Template Provider then uses that to see if sending a new `NewTemplate` message is justified.
It still takes 20-30ms on mainnet to create a template, withou
...
(https://github.com/bitcoin/bitcoin/pull/30443#issuecomment-2236950771)
@sdaftuar @sipa I made a branch that combines #28676 with #29432: https://github.com/Sjors/bitcoin/tree/sv2-cluster
The main commit is https://github.com/bitcoin/bitcoin/commit/f646977ddac0622889d484683ae59c262d50d8dc. It slightly changes the `waitFeesChanged()` interface. The implementation now frequently creates a new template.
The Template Provider then uses that to see if sending a new `NewTemplate` message is justified.
It still takes 20-30ms on mainnet to create a template, withou
...