Bitcoin Core Github
44 subscribers
120K links
Download Telegram
πŸ‘ TheCharlatan approved a pull request: "guix: bump time-machine to 7bf1d7aeaffba15c4f680f93ae88fbef25427252"
(https://github.com/bitcoin/bitcoin/pull/30609#pullrequestreview-2228473719)
Nice, ACK eca20bead2daec4898ecf608cdeb74a73766a5f4

Guix builds (aarch64):
```
d079858fb1bc526217ee06f312d97a56c34986440e5f9e108af66eaecacea073 guix-build-eca20bead2da/output/aarch64-linux-gnu/SHA256SUMS.part
2db780ffe39210a3ba113e52362d94840449218ac1747e3a3484606cc36acead guix-build-eca20bead2da/output/aarch64-linux-gnu/bitcoin-eca20bead2da-aarch64-linux-gnu-debug.tar.gz
b56b602bd87e73b11a6b68147c52c6dfa53f0ec4bac52ac749765025e7b43bc9 guix-build-eca20bead2da/output/aarch64-linux-gnu/b
...
πŸ’¬ achow101 commented on pull request "validation: assumeutxo params mainnet":
(https://github.com/bitcoin/bitcoin/pull/28553#issuecomment-2276350075)
Looks like there's a silent merge conflict:

```
kernel/chainparams.cpp: In constructor β€˜CMainParams::CMainParams()’:
kernel/chainparams.cpp:194:9: error: no match for β€˜operator=’ (operand types are β€˜std::__debug::vector<AssumeutxoData>’ and β€˜<brace-enclosed initializer list>’)
194 | };
| ^
In file included from /usr/include/c++/11/vector:76,
from /usr/include/c++/11/functional:62,
from /usr/include/c++/11/pstl/glue_algorithm_def
...
πŸ’¬ theStack commented on pull request "assumeutxo: Drop block height from metadata":
(https://github.com/bitcoin/bitcoin/pull/30598#issuecomment-2276350922)
Concept ACK, now that https://github.com/bitcoin/bitcoin/pull/30516 is history.

While I still think having the block height in the meta-data could have potentially been useful for external tooling, not having it is also not terrible: it will be very likely somehow included in the filename of popular distributed snapshots (so users can at least see it), and for tools that don't have access to the block index, it can at least be implicitly determined by tracking the maximum of all UTXO entries'
...
πŸ’¬ Sjors commented on pull request "Stratum v2 Template Provider (take 3)":
(https://github.com/bitcoin/bitcoin/pull/29432#issuecomment-2276354835)
Rebased after testnet4 landed. Add the two issues to the PR description.
πŸ“ sipa opened a pull request: "validation: do not wipe utxo cache for stats/scans/snapshots"
(https://github.com/bitcoin/bitcoin/pull/30610)
Since #28280, the cost of a non-wiping sync of the UTXO cache is only proportional to the number of dirty entries, rather than proportional to the size of the entire cache. Because of that, there is no reason to perform a wiping flush in cache the contents of the cache is still useful.

Split the `FlushStateMode::ALWAYS` mode into a `FORCE_SYNC` (non-wiping) and a `FORCE_FLUSH` (wiping), and then use the former in `scantxoutset`, `gettxoutsetinfo`, and in the currently-unused `CreateUTXOSnapsh
...
πŸ’¬ sipa commented on pull request "validation: do not wipe utxo cache for stats/scans/snapshots":
(https://github.com/bitcoin/bitcoin/pull/30610#discussion_r1710008795)
It is unclear to me what the desired behavior is here, so I opted not to change it, but opinions welcome.
πŸ’¬ Sjors commented on pull request "validation: assumeutxo params mainnet":
(https://github.com/bitcoin/bitcoin/pull/28553#issuecomment-2276376412)
Rebased due to #30560 and replaced `uint256S("0x...` with `uint256{"...`, as well as using `consteval_ctor` for the block hash, and `m_chain_tx_count`.
πŸ€” murchandamus reviewed a pull request: "wallet: Keep track of the wallet's own transaction outputs in memory"
(https://github.com/bitcoin/bitcoin/pull/27286#pullrequestreview-2225335817)
ReACK 3b6ac646ffea930ae7d8c5d93acaba3587cc9dcd
πŸ’¬ murchandamus commented on pull request "wallet: Keep track of the wallet's own transaction outputs in memory":
(https://github.com/bitcoin/bitcoin/pull/27286#discussion_r1707091091)
In d6f67e97b7ec0b5fdc39e5ad52e0c03dd0e03fd1 wallet: Keep track of transaction outputs owned by the wallet:

It would have helped me if this function had some documentation. Also, "Wallet transaction transaction outputs" feels a bit redundant, and the name `RefreshWalletTxTXOs(…)` is extremely similar to `RefreshAllTXOs()` that gets introduced in the later commit "wallet: Recalculate the wallet's txos after any imports". If you have to touch this again, perhaps consider calling this function `C
...
πŸ’¬ murchandamus commented on pull request "wallet: Keep track of the wallet's own transaction outputs in memory":
(https://github.com/bitcoin/bitcoin/pull/27286#discussion_r1707520564)
In 9d3762913ba02d1f6cd3b42889c2c3b31f03e90c "wallet: Recalculate the wallet's txos after any imports":

Perhaps add a documentation comment here along the lines of:

```suggestion
/** Inspects every wallet transaction and caches all outputs that belong to our wallet. */
void RefreshAllTXOs() EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
```
πŸ’¬ murchandamus commented on pull request "Testnet4 including PoW difficulty adjustment fix":
(https://github.com/bitcoin/bitcoin/pull/29775#issuecomment-2276397742)
@zawy12: I think this gets a bit off-topic here, perhaps it would be better to post about your timewarp scenario to the mailing list or Delving Bitcoin.
πŸ“ andrewtoth opened a pull request: "validation: write chainstate to disk every hour"
(https://github.com/bitcoin/bitcoin/pull/30611)
Since #28233, periodically writing the chainstate to disk every 24 hours does not clear the dbcache. Since #28280, periodically writing the chainstate to disk is proportional only to the amount of dirty entries in the cache. Due to these changes, it is no longer beneficial to only write the chainstate to disk every 24 hours. The periodic flush interval was necessary because every write of the chainstate would clear the dbcache. Now, we can get rid of the periodic flush interval and simply write
...
βœ… andrewtoth closed a pull request: "validation: sync chainstate to disk after syncing to tip"
(https://github.com/bitcoin/bitcoin/pull/15218)
πŸ’¬ andrewtoth commented on pull request "validation: sync chainstate to disk after syncing to tip":
(https://github.com/bitcoin/bitcoin/pull/15218#issuecomment-2276409584)
Closing in favor of https://github.com/bitcoin/bitcoin/pull/30611.
πŸ€” ryanofsky reviewed a pull request: "multiprocess: Add IPC wrapper for Mining interface"
(https://github.com/bitcoin/bitcoin/pull/30510#pullrequestreview-2228236248)
Updated 6cccb436557c58e0ccd21ffe0eaf31058f5cb799 -> c538ec69f266b51c893a374a4bb82796ede3d7cb ([`pr/mine-types.3`](https://github.com/ryanofsky/bitcoin/commits/pr/mine-types.3) -> [`pr/mine-types.4`](https://github.com/ryanofsky/bitcoin/commits/pr/mine-types.4), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/mine-types.3..pr/mine-types.4)) with review suggestions.

Thanks for the review and thoughtful questions. I wrote detailed answers in line but I will reuse the text & diagram in
...
πŸ’¬ ryanofsky commented on pull request "multiprocess: Add IPC wrapper for Mining interface":
(https://github.com/bitcoin/bitcoin/pull/30510#discussion_r1709774619)
re: https://github.com/bitcoin/bitcoin/pull/30510#discussion_r1708937001

> I'm having a hard time reading this TODO. Can you try and reformulate it?

I'm removed the TODO since it's a low-priority efficiency improvement. Basically, there are 2 ways to call `CustomReadField` and there are 2 ways to deserialize certain objects, and the code is not currently choosing the most efficient way to deserialize based on the way it is called.

To explain further, the `CustomReadField` function is re
...
πŸ’¬ ryanofsky commented on pull request "multiprocess: Add IPC wrapper for Mining interface":
(https://github.com/bitcoin/bitcoin/pull/30510#discussion_r1710015402)
re: https://github.com/bitcoin/bitcoin/pull/30510#discussion_r1708943992

> Typo nit: s/canproto/capnproto/ (here and elsewhere).

Thanks, fixed
πŸ’¬ ryanofsky commented on pull request "multiprocess: Add IPC wrapper for Mining interface":
(https://github.com/bitcoin/bitcoin/pull/30510#discussion_r1709823817)
re: https://github.com/bitcoin/bitcoin/pull/30510#discussion_r1708996040

> Not directly related to this PR, but did these type hook methods get renamed since the docs were written? https://github.com/ryanofsky/bitcoin/blob/pr/ipc/doc/design/multiprocess.md#type-hooks-in-srcipccapnp-typesh

Good catch, updated the docs to mention these. The custom message hooks have actually been around a while but I moved them from bitcoin code to libmultiprocess code recently in https://github.com/chaincod
...
πŸ’¬ ryanofsky commented on pull request "multiprocess: Add IPC wrapper for Mining interface":
(https://github.com/bitcoin/bitcoin/pull/30510#discussion_r1709911230)
re: https://github.com/bitcoin/bitcoin/pull/30510#discussion_r1709048212

> I'm a bit confused by this file. It is only included by `mining.capnp`, but in turn includes the auto-generated header from `mining.capnp` here. What is the purpose of this? Couldn't the capnp file just directly include `interfaces/mining.h`? At least doing so compiles for me.

I simplified this as suggested. I had just copied these mining interface files from one of the other interfaces (probably node or chain) and
...
πŸ’¬ ryanofsky commented on pull request "multiprocess: Add IPC wrapper for Mining interface":
(https://github.com/bitcoin/bitcoin/pull/30510#discussion_r1709972901)
re: https://github.com/bitcoin/bitcoin/pull/30510#discussion_r1709096542

> I think it would be nice to test some more invariants here:

Thanks, added