💬 Ayush170-Future commented on pull request "fuzz: wallet, add target for Spend":
(https://github.com/bitcoin/bitcoin/pull/28236#issuecomment-1793499068)
- Force pushed rebasing on current master branch.

👍 Ayush170-Future approved a pull request: "fuzz: add target for `DescriptorScriptPubKeyMan`"
(https://github.com/bitcoin/bitcoin/pull/28578#pullrequestreview-1713827504)
ACK

Reviewed the whole code. Looks great to me!

⚠️ Gitsarry opened an issue: "Possibility to dump all runtime parameter values"
(https://github.com/bitcoin/bitcoin/issues/28790)
### Please describe the feature you'd like to see added.

I would like to be able to see all parameters and it's values that bitcoind "knows" when it is running.
That are at least those parameters you can set in `bitcoin.conf` file, e.g. `upnp`, `disablewallet` etc.

My use case is with bitcoind `v26.0.0rc2` I want to test enabling v2 transport with `v2transport=1`.

Since I have since then not one v2 connection (which is perfectly reasonable, I guess, since it is a new feature disabled by
...

💬 kashifs commented on pull request "net: improves addnode / m_added_nodes logic":
(https://github.com/bitcoin/bitcoin/pull/28155#issuecomment-1793511504)
> > tACK [0420f9](https://github.com/bitcoin/bitcoin/commit/0420f99f429ce2382057e101859067f40de47be0)
>
>
>
> Thanks for reviewing! Would you mind sharing what did you test and how?
>
>
>
> @vasild would you mind re-acking this when you have some time?

I pulled the branch, compiled from source on my Mac, and ran:

./src/test/test_bitcoin -t net_peer_connection_tests -l message -- -printtoconsole=1

and

`./src/test/test_bitcoin -t net_peer_connection_tests -l all -- -printtoconsole=1`

I
...

💬 achow101 commented on issue "Possibility to dump all runtime parameter values":
(https://github.com/bitcoin/bitcoin/issues/28790#issuecomment-1793511718)
All interpreted config options are logged to the debug.log file.

💬 Gitsarry commented on issue "Possibility to dump all runtime parameter values":
(https://github.com/bitcoin/bitcoin/issues/28790#issuecomment-1793514712)
@achow101 Thanks, I did not know that, I guess that feature was added some time after I last inspected the debug log in detail (some years ago? ;). I guess you mean lines like these?
```
2023-08-08T17:44:49Z Config file: /home/user/.bitcoin/bitcoin.conf
2023-08-08T17:44:49Z Config file arg: i2psam="127.0.0.1:7656"
2023-08-08T17:44:49Z Config file arg: listen="1"
```
That solves my particular use case, but maybe it's worth to consider dumping all runtime values as written in OP before closi
...

💬 Ayush170-Future commented on pull request "fuzz: wallet, add target for Spend":
(https://github.com/bitcoin/bitcoin/pull/28236#issuecomment-1793526872)
- Force-pushed updating the `CRecipient` wrt [28246](https://github.com/bitcoin/bitcoin/pull/28246).

All CI checks passed now 💪

📝 maaku opened a pull request: "snapshots: don't core dump when running -checkblockindex after `loadtxoutset`"
(https://github.com/bitcoin/bitcoin/pull/28791)
…xoutset`

<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements or new test
...

👍 ryanofsky approved a pull request: "depends: Bump to capnproto-c++-1.0.1"
(https://github.com/bitcoin/bitcoin/pull/28735#pullrequestreview-1713852026)
Code review ACK 3333f14efac815ba3c885398a6795c7e8ce68d08

Am wondering if we know why the guix build succeeds and CI passes if the ubuntu mingw build fails manually. Is that expected? Would it make any sense to change what platforms CI covers, or is the failing build platform pretty close to the ones that succeed and the failure is just a quirk?

💬 sr-gi commented on pull request "net: improves addnode / m_added_nodes logic":
(https://github.com/bitcoin/bitcoin/pull/28155#issuecomment-1793564549)
> > > tACK [0420f9](https://github.com/bitcoin/bitcoin/commit/0420f99f429ce2382057e101859067f40de47be0)
> >
> >
> >
> > Thanks for reviewing! Would you mind sharing what did you test and how?
> >
> >
> >
> > @vasild would you mind re-acking this when you have some time?
>
> I pulled the branch, compiled from source on my Mac, and ran:
>
> `./src/test/test_bitcoin -t net_peer_connection_tests -l message -- -printtoconsole=1`
>
> and
>
> `./src/test/test_bitcoin -t net_peer_connection_te
...

📝 fjahr opened a pull request: "Embedding ASMap files as binary dump header file"
(https://github.com/bitcoin/bitcoin/pull/28792)
Looking for conceptual feedback!

This is one of two options for how we may embed ASMap files into Bitcoin Core releases. This approach here uses a binary dump of the file which is then committed to the source code as a header file. The alternative approach is not having the data in the source code but only adding it during the build phase of a release. I initially favored the second approach but it seems harder to achieve than I originally expected and I have also started to see the downsides
...

📝 fjahr opened a pull request: "contrib: Add asmap-tool"
(https://github.com/bitcoin/bitcoin/pull/28793)
This adds `asmap.py` and `asmap-tool.py` from sipa's `nextgen` branch: https://github.com/sipa/asmap/tree/nextgen

The motivation is that we should maintain the tooling for de- and encoding asmap files within the bitcoin core repository because it is not possible to use an asmap file that is not encoded.

We already had an earlier version of `asmap.py` within the seeds contrib tools. The newer version only had a small amount of changes and is still compatible, so the old version is removed
...

⚠️ fjahr opened an issue: "Embedded ASMap data Tracking Issue"
(https://github.com/bitcoin/bitcoin/issues/28794)
This issue will be updated continuously to reflect the process on embedding ASMap data in Bitcoin Core, so asmap can be used without procuring an asmap file first. Having the asmap data available embedded is a requirement for potentially using it by default in the future.

**PRs Ready for review:**
- https://github.com/bitcoin/bitcoin/pull/27581
- https://github.com/bitcoin/bitcoin/pull/28793

**Looking for conceptual review:**
- https://github.com/bitcoin/bitcoin/pull/28792

**TODOs**
...

💬 ishaanam commented on pull request "wallet: track mempool conflicts with wallet transactions":
(https://github.com/bitcoin/bitcoin/pull/27307#discussion_r1382482637)
I agree it is better to track conflicts on a transaction level. I've implemented it this way now and it looks much better.

💬 sipa commented on pull request "Embedding ASMap files as binary dump header file":
(https://github.com/bitcoin/bitcoin/pull/28792#issuecomment-1793590960)
Would it make sense to only check in the asmap.dat file into the repository, and then have say a python script that converts it to a .h at compile time (similar to the approach used in the `src/test/data` directory)? That would mean there is also a directly-manipulable file in the repository (that can be used with `asmap-tool` etc).

💬 sipa commented on pull request "contrib: Add asmap-tool":
(https://github.com/bitcoin/bitcoin/pull/28793#discussion_r1382486877)
This was never implemented, it should probably be dropped.

⚠️ gianlucamazza opened an issue: "bitcoin core crashes and restarts syncing from beginning "
(https://github.com/bitcoin/bitcoin/issues/28795)
### Is there an existing issue for this?

- [X] I have searched the existing issues

### Current behaviour

I am consistently encountering a `LevelDB read failure: Corruption: block checksum mismatch` error when attempting to synchronize the Bitcoin blockchain on multiple Raspberry Pi devices.
The corruption error happens always around 2016-01-28T16:10:24Z as far I remember. Any idea why this happens?

### Expected behaviour

the full node should fully syncronize

### Steps to reproduce

bitcoi
...

🤔 theStack reviewed a pull request: "wallet: Implement independent BDB parser"
(https://github.com/bitcoin/bitcoin/pull/26606#pullrequestreview-1713893741)
Concept ACK

As a follow-up, it might be nice if we could somehow add coverage for the "other endianness" code path in the tests. Did that manually with the following patch (creates the BDB database in big-endian format, while I'm running a little-endian system):
```diff
diff --git a/src/wallet/bdb.cpp b/src/wallet/bdb.cpp
index 9ea43ca67c..945e12ebdb 100644
--- a/src/wallet/bdb.cpp
+++ b/src/wallet/bdb.cpp
@@ -389,6 +389,7 @@ void BerkeleyDatabase::Open()
}

...

💬 theStack commented on pull request "wallet: Implement independent BDB parser":
(https://github.com/bitcoin/bitcoin/pull/26606#discussion_r1382488042)
(in commit a02db44419148984d726f95fd66030b9a40fc0bb) readability nit: could also use `std::get` here, as the RHS of the OR conditoin is only executed if the variant holds the right data type:
```suggestion
if (!std::holds_alternative<DataRecord>(page.records.at(0)) || std::get<DataRecord>(page.records.at(0)).data != SUBDATABASE_NAME) {
```
(same for the two `page.records.at(1)` instances a few lines below)

💬 theStack commented on pull request "wallet: Implement independent BDB parser":
(https://github.com/bitcoin/bitcoin/pull/26606#discussion_r1382487576)
(in commit b7b90daf46ae840d9ab089ac438f71f7b79dbf5a) could avoid duplicate std::map lookup by using `.find` instead:
```suggestion
const auto it{m_database.m_records.find(key_data)};
if (it == m_database.m_records.end()) {
return false;
}
auto val = it->second;
```