Bitcoin Core Github
43 subscribers
122K links
Download Telegram
🤔 mzumsande reviewed a pull request: "Fix rpc_getblockfrompeer intermittency by introducing 'getblockfileinfo' RPC command"
(https://github.com/bitcoin/bitcoin/pull/27770#pullrequestreview-1449612463)
> Furthermore, what theStack analysis really says is that pruneblockchain result is not accurate. Which is the real issue for me. The RPC command result description says that it returns the "height of the last block pruned", which could not be true as blocks could have been stored out of order.
So retrieving block height 249 as "last block pruned" doesn't mean that block 248 has been pruned from disk.

But is it really incorrect that the "last block pruned" is 249? Yes, this doesn't imply tha
...
🚀 fanquake merged a pull request: "Fix `#include`s in `src/wallet`"
(https://github.com/bitcoin/bitcoin/pull/27759)
👍 fanquake approved a pull request: "build: disable boost multi index safe mode in debug mode"
(https://github.com/bitcoin/bitcoin/pull/27724#pullrequestreview-1449677075)
ACK 59c89447499bd9d6202269879555b8bc37373aa2
💬 furszy commented on pull request "Fix rpc_getblockfrompeer intermittency by introducing 'getblockfileinfo' RPC command":
(https://github.com/bitcoin/bitcoin/pull/27770#issuecomment-1567299154)
> But is it really incorrect that the "last block pruned" is 249? Yes, this doesn't imply that all blocks before have been pruned as well - but the RPC description isn't claiming that.

That would be the devil's lawyer argument. It works for us, but not for our users.
Let's agree that while it's not clearly stated, anyone reading the RPC result description would understand that all blocks prior the returned height were pruned.

> But I think that introducing any new RPC should primarily hav
...
💬 fanquake commented on pull request "ci: compile Clang and compiler-rt in msan jobs":
(https://github.com/bitcoin/bitcoin/pull/27737#discussion_r1209434809)
> Looks like it is only passing CI because Cirrus seems to be ignoring the 2CPU/8GB limit and just uses 4/15 unconditionally for ci image builds?

@fkorotkov is this what should be happening, and can we rely on this behaviour into the future?
👍 TheCharlatan approved a pull request: "lint: stop ignoring LIEF imports"
(https://github.com/bitcoin/bitcoin/pull/27507#pullrequestreview-1449687958)
ACK 015cc5e588fa25f65f6ea2ed03701def8dfd5444
fanquake closed an issue: "CPU DoS on mainnet in debug mode"
(https://github.com/bitcoin/bitcoin/issues/27586)
🚀 fanquake merged a pull request: "build: disable boost multi index safe mode in debug mode"
(https://github.com/bitcoin/bitcoin/pull/27724)
🚀 fanquake merged a pull request: "lint: stop ignoring LIEF imports"
(https://github.com/bitcoin/bitcoin/pull/27507)
📝 fanquake opened a pull request: "[25.x] build: disable boost multi index safe mode"
(https://github.com/bitcoin/bitcoin/pull/27775)
Backports https://github.com/bitcoin/bitcoin/pull/27724 to `25.x`.
💬 fanquake commented on pull request "build: disable boost multi index safe mode in debug mode":
(https://github.com/bitcoin/bitcoin/pull/27724#issuecomment-1567320438)
Backported to 25.x in #27775.
💬 LarryRuane commented on pull request "util: generalize accounting of system-allocated memory in pool resource":
(https://github.com/bitcoin/bitcoin/pull/27748#discussion_r1209457660)
I copied this function from `memusage.h` ([here](https://github.com/bitcoin/bitcoin/blob/6cf47a8f44f96099a84e5c6e628e3499045e024d/src/memusage.h#L52)). I'm hoping not to duplicate this function, even though it's pretty small. `pool.h` can't include `memusage.h` because that creates a circular dependency. I'll try to figure out a way to improve this and also of course I welcome suggestions.

Anyway, if we need to duplicate this function, I'll remove this comment, I agree it's not useful.
💬 joostjager commented on issue "Allow accepting non-standard transactions on mainnet via local rpc":
(https://github.com/bitcoin/bitcoin/issues/27768#issuecomment-1567353059)
Standardising the annex would be great, but how likely is it that that will be done in policy only without a soft fork? I haven't followed L1 closely, but it seems that soft forks aren't an easy thing to pull off.
💬 MarcoFalke commented on pull request "Fix `#include`s in `src/wallet`":
(https://github.com/bitcoin/bitcoin/pull/27759#issuecomment-1567357663)
May be good to report the crash upstream if you can reduce the reproducer
🤔 hebasto reviewed a pull request: "lint: stop ignoring LIEF imports"
(https://github.com/bitcoin/bitcoin/pull/27507#pullrequestreview-1449741321)
Post-merge ACK 015cc5e588fa25f65f6ea2ed03701def8dfd5444.
💬 pablomartin4btc commented on pull request "httpserver, rest: improving URI validation":
(https://github.com/bitcoin/bitcoin/pull/27253#issuecomment-1567380170)
Updates:
- Working on `fuzzer` failure.
💬 brunoerg commented on pull request "p2p: Diversify automatic outbound connections with respect to networks":
(https://github.com/bitcoin/bitcoin/pull/27213#discussion_r1209501764)
We want to protect a peer if it's the only one for its network. However, we can run a node supporting just one network. In this case, I suppose that calling `GetFullOutboundAndManualCount` might be unnecessary, we would iterating the whole `m_nodes` with no need.

In this case, I think we could have a function like `SupportsMultipleNetworks` to check whether we support more than one network, e.g.:
```cpp
bool CConnman::SupportsMultipleNetworks() const
{
int count = 0;
for (int n =
...
💬 brunoerg commented on pull request "p2p: Diversify automatic outbound connections with respect to networks":
(https://github.com/bitcoin/bitcoin/pull/27213#issuecomment-1567445222)
Just a suggestion, but I think we could cover `GetFullOutboundAndManualCount` in fuzz.

```diff
diff --git a/src/test/fuzz/connman.cpp b/src/test/fuzz/connman.cpp
index f81658b83..f5867f8b4 100644
--- a/src/test/fuzz/connman.cpp
+++ b/src/test/fuzz/connman.cpp
@@ -128,4 +128,6 @@ FUZZ_TARGET_INIT(connman, initialize_connman)
(void)connman.GetTotalBytesSent();
(void)connman.GetTryNewOutboundPeer();
(void)connman.GetUseAddrmanOutgoing();
+ const Network net{fuzzed_data
...
📝 xeroxparc opened a pull request: "BITCOIN EMERGENCY MODE - Lets talk about ethics and integrity"
(https://github.com/bitcoin/bitcoin/pull/27776)
Im not saying my idea is the solution. I wrote it so that the internal team can have a real honest conversation about the ethic and integrity of the project.

<!--
*** 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 a
...
fanquake closed a pull request: "BITCOIN EMERGENCY MODE - Lets talk about ethics and integrity"
(https://github.com/bitcoin/bitcoin/pull/27776)