Bitcoin Core Github
44 subscribers
121K links
Download Telegram
💬 TheCharlatan commented on pull request "blockstorage: Return on fatal flush errors":
(https://github.com/bitcoin/bitcoin/pull/27866#discussion_r1254840912)
> Maybe your goal is to enhance libbitcoinkernel applications ability to handle and report errors, and my goal is to improve clarify of validation code and find to potential bugs in it.

I think that hits it on the nail. In any case I updated the PR description with much of the body of the commit message to make it clearer what I am trying to achieve here and why I'd consider it an improvement.
📝 theuni opened a pull request: "wallet: don't include bdb files from our headers"
(https://github.com/bitcoin/bitcoin/pull/28039)
Only `#include` upstream bdb headers from our cpp files.

It's generally good practice to avoid including 3rd party deps in headers as otherwise they tend to sneak into new compilation units. IMO this makes for a nice cleanup.

There's a good bit of code movement here, but each commit is small and _should_ be obviously correct.

Note: in the future, the buildsystem can add the bdb include path for `bdb.cpp` and `salvage.cpp` only, rather than all wallet sources.
📝 theuni opened a pull request: "wallet: sqlite: don't include sqlite files from our headers"
(https://github.com/bitcoin/bitcoin/pull/28040)
Only `#include` upstream sqlite headers from our cpp files.

Like #28039 but simpler :)
💬 Xekyo commented on pull request "wallet: Keep track of the wallet's own transaction outputs in memory":
(https://github.com/bitcoin/bitcoin/pull/27286#issuecomment-1624254517)
Concept ACK
📝 1proprogrammerchant opened a pull request: "Merge pull request #1 from bitcoin/master"
(https://github.com/bitcoin/bitcoin/pull/28041)
pull_1

<!--
*** 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 tests t
...
hebasto closed a pull request: "Merge pull request #1 from bitcoin/master"
(https://github.com/bitcoin/bitcoin/pull/28041)
📝 hebasto locked a pull request: "."
(https://github.com/bitcoin/bitcoin/pull/28041)
pull_1

<!--
*** 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 tests t
...
💬 mzumsande commented on issue "EstimateMedianVal returns higher fee for higher confTarget":
(https://github.com/bitcoin/bitcoin/issues/20725#issuecomment-1624279055)
> @ismaelsadeeq Can you try with the commit specified in OP as well? If it fails, can you try bisecting?

Maybe not necessary:
There are newer crashes reported in #23165 (which seems to be the same problem). The seed `7288975409214300634` from https://api.cirrus-ci.com/v1/task/6110951150190592/logs/ci.log leads to a crash on current master (the test passes with #21161 rebased on master).
💬 achow101 commented on pull request "wallet: address book migration bug fixes":
(https://github.com/bitcoin/bitcoin/pull/28038#issuecomment-1624292395)
ACK 7ecc29a0b7a23d8f5d3c1e6a0dad29b3ad839eb9

Nice catch!
🚀 ryanofsky merged a pull request: "kernel: Rm ShutdownRequested and AbortNode from validation code."
(https://github.com/bitcoin/bitcoin/pull/27861)
💬 achow101 commented on pull request "wallet: sqlite: don't include sqlite files from our headers":
(https://github.com/bitcoin/bitcoin/pull/28040#issuecomment-1624315734)
ACK bea9fc2600635020fd28ec7a6613c92a6f349a86
💬 furszy commented on pull request "index: make startup more efficient":
(https://github.com/bitcoin/bitcoin/pull/27607#discussion_r1254925898)
Sure, applied. Thanks!
💬 sipa commented on pull request "Add feerate histogram to getmempoolinfo":
(https://github.com/bitcoin/bitcoin/pull/21422#issuecomment-1624321895)
@santochibtc "Feerate histogram" here is a bytes vs fee diagram, not a #tx vs fee diagram.
💬 furszy commented on pull request "index: make startup more efficient":
(https://github.com/bitcoin/bitcoin/pull/27607#discussion_r1254926452)
done
💬 furszy commented on pull request "index: make startup more efficient":
(https://github.com/bitcoin/bitcoin/pull/27607#discussion_r1254926965)
removed.
💬 achow101 commented on pull request "wallet: don't include bdb files from our headers":
(https://github.com/bitcoin/bitcoin/pull/28039#issuecomment-1624325625)
ACK 2d09d0f50408f9c522b9efa4f386072502c7b3d1

Changes are primarily moves and appear otherwise obviously correct.
💬 mzumsande commented on issue "qa: Intermittent failure in `feature_fee_estimation.py`":
(https://github.com/bitcoin/bitcoin/issues/23165#issuecomment-1624334379)
Just noting here too (I also posted in #20725) that failures can be reproduced by using the PRNG seed from the log (`7288975409214300634` in the most recent one) and running with `--randomseed`. So it would be good to include that seed in future reports - the earlier ones are no longer accessible from cirrus.
💬 jonatack commented on pull request "p2p: Drop m_recently_announced_invs bloom filter":
(https://github.com/bitcoin/bitcoin/pull/27675#discussion_r1254974082)
Thanks, makes sense, and I see that your change makes the conditional here the same as in `ProcessMessage::TX` followed by the same comment.

```cpp
if (peer->m_wtxid_relay && txid != wtxid) {
// Insert txid into m_tx_inventory_known_filter, even for
// wtxidrelay peers. This prevents re-adding of
// unconfirmed parents to the recently_announced
// filter, when a child tx is requested. See
// ProcessGetData().

...
🤔 ismaelsadeeq reviewed a pull request: "Fee estimation: extend bucket ranges consistently"
(https://github.com/bitcoin/bitcoin/pull/21161#pullrequestreview-1517440130)
The issue this is solving occurs intermittently.
I tested the test failed on recent master c325f0f with [seed](https://github.com/bitcoin/bitcoin/issues/20725#issuecomment-1624279055) from @mzumsande and passed on this PR rebased on master head c325f0f.

Did not finish understanding the PR yet