💬 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
...
(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 💪
(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
...
(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?
(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
...
(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
...
(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
...
(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**
...
(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.
(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).
(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.
(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
...
(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()
}
...
(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)
(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;
```
(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;
```
📝 hebasto opened a pull request: "ci: Drop no longer needed "Fix Visual Studio installation" step"
(https://github.com/bitcoin/bitcoin/pull/28796)
The underlying issue has been [fixed](https://github.com/actions/runner-images/pull/8686) in the image version 20231029.
(https://github.com/bitcoin/bitcoin/pull/28796)
The underlying issue has been [fixed](https://github.com/actions/runner-images/pull/8686) in the image version 20231029.
💬 pinheadmz commented on issue "bitcoin core crashes and restarts syncing from beginning ":
(https://github.com/bitcoin/bitcoin/issues/28795#issuecomment-1793707433)
You're running Bitcoin Core on a Raspberry Pi which can work with the right setup. Looks like you are storing blocks on an external drive of some kind, but the chainstate database might still be stored on the MicroSD card (?) and got corrupted.
It's a little hard to tell from your output what kind of external drive you have but assuming its a good quality SSD connected by USB3 to the Pi, you might consider mounting it directly to `/home/pi/.bitcoin` (this is what I do). Then all block, index,
...
(https://github.com/bitcoin/bitcoin/issues/28795#issuecomment-1793707433)
You're running Bitcoin Core on a Raspberry Pi which can work with the right setup. Looks like you are storing blocks on an external drive of some kind, but the chainstate database might still be stored on the MicroSD card (?) and got corrupted.
It's a little hard to tell from your output what kind of external drive you have but assuming its a good quality SSD connected by USB3 to the Pi, you might consider mounting it directly to `/home/pi/.bitcoin` (this is what I do). Then all block, index,
...
💬 Retropex commented on pull request "datacarriersize: Match more datacarrying":
(https://github.com/bitcoin/bitcoin/pull/28408#issuecomment-1793719771)
> Node runners need a builtin option to ignore all modern forms of datacarrying so they don't have to resort to manually patching their nodes.
In addition, it is important to note that if developers do not help node runners defend themselves against this attack, they may have to resort to misappropriate means to strengthen their defense. Although the use of a pre-SegWit node is a defense option, it can have harmful consequences for the entire network.
(https://github.com/bitcoin/bitcoin/pull/28408#issuecomment-1793719771)
> Node runners need a builtin option to ignore all modern forms of datacarrying so they don't have to resort to manually patching their nodes.
In addition, it is important to note that if developers do not help node runners defend themselves against this attack, they may have to resort to misappropriate means to strengthen their defense. Although the use of a pre-SegWit node is a defense option, it can have harmful consequences for the entire network.
💬 S3RK commented on pull request "wallet: Have the wallet store the key for automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/26728#issuecomment-1793751225)
Nice, I'm going to re-review this once I'm done with 25273
(https://github.com/bitcoin/bitcoin/pull/26728#issuecomment-1793751225)
Nice, I'm going to re-review this once I'm done with 25273
💬 andrewtoth commented on pull request "p2p: adaptive connections services flags":
(https://github.com/bitcoin/bitcoin/pull/28170#issuecomment-1793773276)
ACK 5628ac55be42c5450c6817ba8dcfe463ceda32a9
(https://github.com/bitcoin/bitcoin/pull/28170#issuecomment-1793773276)
ACK 5628ac55be42c5450c6817ba8dcfe463ceda32a9