Bitcoin Core Github
44 subscribers
120K links
Download Telegram
💬 vasild commented on pull request "log: Mitigate disk filling attacks by rate limiting LogPrintf, LogInfo, LogWarning, LogError":
(https://github.com/bitcoin/bitcoin/pull/32604#discussion_r2348333882)
Done in https://github.com/bitcoin/bitcoin/pull/33388
💬 ajtowns commented on pull request "mining: add applySolution() to interface":
(https://github.com/bitcoin/bitcoin/pull/33374#discussion_r2348345510)
An OP_TRUE signet would presumably get both for whatever that's worth
💬 fanquake commented on issue "Assertion hit on shutdown of bitcoin-node with connected mining interface client":
(https://github.com/bitcoin/bitcoin/issues/33387#issuecomment-3291201455)
@Sjors
💬 fanquake commented on pull request "test: Fix CLI_MAX_ARG_SIZE issues":
(https://github.com/bitcoin/bitcoin/pull/33243#issuecomment-3291221804)
Backported to 30.x in #33356.
👍 0xB10C approved a pull request: "depends: systemtap 5.3"
(https://github.com/bitcoin/bitcoin/pull/33373#pullrequestreview-3223791227)
ACK 28efd724b47841a16d0630cec4b59563cda54a81

I agree that the package sha256 hash is `966a360fb73a4b65a8d0b51b389577b3c4f92a327e84aae58682103e8c65a69a`. The diff is minimal and looks fine. The guix build hashes match.

```bash
$ find guix-build-$(git rev-parse --short=12 HEAD)/output/ -type f -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum
860c39a3db7eb7e8aaf8165cd6834d2fd87afc69b82b20203a013c7ad7e7b057 guix-build-28efd724b478/output/aarch64-linux-gnu/SHA256SUMS.part
841aea1c04fd996bc437a
...
⚠️ ismaelsadeeq opened an issue: "RFC: Bitcoin Core Node `BlockTemplateManager`"
(https://github.com/bitcoin/bitcoin/issues/33389)
Bitcoin Core Node `BlockTemplateManager` main use should be handling block template creation for other components of the node.
It should use the low-level block assembler to do that. This `BlockTemplateManager` should be initialized with a pointer to the mempool and a `ChainstateManager` reference. It should have access to the default block creation options and be instantiated during node startup and destroyed when the node is shutting down.
This `BlockTemplateManager` should be available via t
...
🤔 Eunovo reviewed a pull request: "log: always print initial signature verification state"
(https://github.com/bitcoin/bitcoin/pull/33336#pullrequestreview-3223525430)
Concept ACK https://github.com/bitcoin/bitcoin/pull/33336/commits/63dc937eea297332ec85c1abbcf3b94b8f74aa85
Left some comments.
💬 Eunovo commented on pull request "log: always print initial signature verification state":
(https://github.com/bitcoin/bitcoin/pull/33336#discussion_r2348447122)
https://github.com/bitcoin/bitcoin/pull/33336/commits/3ddd4c84af2bf0b5d3e403b71f4750b2326118e1: I think it's best to take out this TODO from the commit. You should also adjust the commit message to focus on what this commit is adding specifically.
💬 Eunovo commented on pull request "log: always print initial signature verification state":
(https://github.com/bitcoin/bitcoin/pull/33336#discussion_r2348463061)
https://github.com/bitcoin/bitcoin/pull/33336/commits/3ddd4c84af2bf0b5d3e403b71f4750b2326118e1: A comment here will also be beneficial.
💬 Eunovo commented on pull request "log: always print initial signature verification state":
(https://github.com/bitcoin/bitcoin/pull/33336#discussion_r2348538334)
https://github.com/bitcoin/bitcoin/pull/33336/commits/63dc937eea297332ec85c1abbcf3b94b8f74aa85:
should be "block not on best header chain"
💬 Eunovo commented on pull request "log: always print initial signature verification state":
(https://github.com/bitcoin/bitcoin/pull/33336#discussion_r2348503453)
https://github.com/bitcoin/bitcoin/pull/33336/commits/6222d8cced4828b83a9dde20b9da9df44848b9bd:
Why not `AssumeValid::DISABLED` instead of `AssumeValid::CHECKED` and `AssumeValid::ENABLED` instead of `AssumeValid:SKIPPED`?
I find the current names to be confusing. I think using `AssumeValid::ENABLED` to indicate that assume_valid is enabled is better.
💬 Eunovo commented on pull request "log: always print initial signature verification state":
(https://github.com/bitcoin/bitcoin/pull/33336#discussion_r2348449770)
https://github.com/bitcoin/bitcoin/pull/33336/commits/3ddd4c84af2bf0b5d3e403b71f4750b2326118e1: This should probably still be set to `3` since the 4th node is not used in this commit.
💬 Eunovo commented on pull request "log: always print initial signature verification state":
(https://github.com/bitcoin/bitcoin/pull/33336#discussion_r2348462197)
https://github.com/bitcoin/bitcoin/pull/33336/commits/3ddd4c84af2bf0b5d3e403b71f4750b2326118e1: A comment explaining this check will be beneficial.
💬 Eunovo commented on pull request "log: always print initial signature verification state":
(https://github.com/bitcoin/bitcoin/pull/33336#discussion_r2348540085)
https://github.com/bitcoin/bitcoin/pull/33336/commits/63dc937eea297332ec85c1abbcf3b94b8f74aa85:
consider "block not part of assumevalid chain"
💬 Eunovo commented on pull request "log: always print initial signature verification state":
(https://github.com/bitcoin/bitcoin/pull/33336#discussion_r2348292328)
https://github.com/bitcoin/bitcoin/pull/33336/commits/63dc937eea297332ec85c1abbcf3b94b8f74aa85:
+1 on returning "unknown reason" instead of aborting.
💬 Eunovo commented on pull request "validation: ensure assumevalid is always used during reindex":
(https://github.com/bitcoin/bitcoin/pull/31615#issuecomment-3291494499)
Rebased on master and added a comment explaining why we bother to skip minimumchainwork, even though we expect minimumchainwork to match assumevalid block height most times.
💬 0xB10C commented on issue "v30.0 Testing":
(https://github.com/bitcoin/bitcoin/issues/33368#issuecomment-3291550859)
Running v30.0rc1 on a couple of [peer.observer](https:///public.peer.observer) nodes - will report if I see something suspicious.
👍 ismaelsadeeq approved a pull request: "fuzz: enhance wallet_fees by mocking mempool stuff"
(https://github.com/bitcoin/bitcoin/pull/33210#pullrequestreview-3224092444)
Code review ACK c9a7a198d9e81e99de99a2aaff1687d13d6674e8
with a comment and nit happy to reACK when you amend my suggestion
💬 ismaelsadeeq commented on pull request "fuzz: enhance wallet_fees by mocking mempool stuff":
(https://github.com/bitcoin/bitcoin/pull/33210#discussion_r2348652999)
In "fuzz: mock CBlockPolicyEstimator in wallet_fuzz" ff10a37e99271125a9ece92bae571f7b78fb9e22

It would be nice if `FuzzedBlockPolicyEstimator` were renamed to `MockedBlockPolicyEstimator`.
It could then be moved to `src/test/util/` so that it can be reused by other components that might need a fee rate estimate as well.
💬 ismaelsadeeq commented on pull request "fuzz: enhance wallet_fees by mocking mempool stuff":
(https://github.com/bitcoin/bitcoin/pull/33210#discussion_r2348701041)
In "fuzz: create FeeEstimatorTestingSetup to set fee_estimator" adf67eb21baf39a222b65480e45ae76f093e8f66

nit: It will be nice if we don't have to set the fee estimator and just initialize it in the `FeeEstimatorTestingSetup` constructor, but we have to find a way to pass the fuzz data provider as well.