Bitcoin Core Github
45 subscribers
119K links
Download Telegram
💬 tdb3 commented on pull request "rpc: add `relevant_blocks` to `scanblocks status`":
(https://github.com/bitcoin/bitcoin/pull/30713#issuecomment-2628649148)
Will revisit later
💬 davidgumberg commented on issue "fuzz: oss-fuzz coverage build is failing":
(https://github.com/bitcoin/bitcoin/issues/31770#issuecomment-2628741241)
Bisected to https://github.com/bitcoin/bitcoin/commit/5691fa93c48c5b2c767f19aad5e972e4d760414a (#31661),

Can reproduce doing something like:

```bash
# setup
git clone --depth 1 https://github.com/google/oss-fuzz.git
git clone https://github.com/bitcoin/bitcoin oss-fuzz/bitcoin
git clone --depth 1 https://github.com/bitcoin-core/qa-assets oss-fuzz/bitcoin/assets
cd oss-fuzz
python infra/helper.py build_image bitcoin-core

# check out a commit hash for the local copy
BAD_COMMIT=5691fa93c4
GOOD_
...
💬 jonatack commented on pull request "rpc, logging: return "verificationprogress" of 1 when up to date":
(https://github.com/bitcoin/bitcoin/pull/31177#issuecomment-2628749739)
Thanks for updating -- will review.
💬 vasild commented on pull request "Add waitNext() to BlockTemplate interface":
(https://github.com/bitcoin/bitcoin/pull/31283#discussion_r1938204958)
Ok, it will never generate a template because it will never exit from `waitNext()` (when run from unit tests with mocktime which never advances). "BOOST_REQUIRE(block_template) checks will fail" -- they will never be reached in that case.

What about this (unit tests will use timeout=0):

```
now = now()
deadline = now + timeout;
do {
stuff;
now = now();
} while (now < deadline);
```
it will execute the body of the loop exactly once when given timeout=0 regardless of whether
...
💬 Sjors commented on pull request "Add waitNext() to BlockTemplate interface":
(https://github.com/bitcoin/bitcoin/pull/31283#discussion_r1938233022)
I think it's better explicitly mention the test issue:
1. Less likely for someone to accidentally break it and be confused
2. Maybe someone adds a nice macro one day `RUN_COMMAND_DO_OTHER_THING_IN_NEW_THREAD_AFTER(command, delay)`
💬 Sjors commented on pull request "rpc: add optional blockhash to waitfornewblock, unhide in help":
(https://github.com/bitcoin/bitcoin/pull/30635#issuecomment-2628866683)
Rebased after #31600.
💬 Sjors commented on pull request "test: clarify timewarp grace period griefing attack":
(https://github.com/bitcoin/bitcoin/pull/31725#issuecomment-2628867551)
#31600 landed, ready for review
👋 Sjors's pull request is ready for review: "test: clarify timewarp grace period griefing attack"
(https://github.com/bitcoin/bitcoin/pull/31725)
💬 Sjors commented on pull request "mining: bugfix: Fix duplicate coinbase tx weight reservation":
(https://github.com/bitcoin/bitcoin/pull/31384#discussion_r1938236496)
Miners don't add the header though.
💬 Sjors commented on pull request "mining: bugfix: Fix duplicate coinbase tx weight reservation":
(https://github.com/bitcoin/bitcoin/pull/31384#discussion_r1938237527)
Indeed, this could be `MAX_BLOCK_WEIGHT`, but definitely not `max_block_weight`.

The clamping does not prevent someone from creating a 4MB coinbase transaction while otherwise not mining anything by setting `-maxblockweight=0`. As nonsensical as that might be, but dropping `-blockmaxweight` seems better to discuss in a followup.
💬 Sjors commented on pull request "mining: bugfix: Fix duplicate coinbase tx weight reservation":
(https://github.com/bitcoin/bitcoin/pull/31384#issuecomment-2628873833)
@pinheadmz wrote:

> some mining pools to squeeze an extra 4000 WU of transaction data into new blocks

@fjahr wrote:

> Only an extra 2000 WU, right?

Just 2000 extra WU. They could already push the effective reservation to 4000 by setting `blockmaxweight=4000000` and there's a minimum value of `-blockreservedweight=2000`.

@ismaelsadeeq wrote:

> No, the current behavior on master is that we always reserve 8,000 WU, regardless of what blockmaxweight is. This is actually how I disco
...
💬 Sjors commented on pull request "mining: bugfix: Fix duplicate coinbase tx weight reservation":
(https://github.com/bitcoin/bitcoin/pull/31384#issuecomment-2628877288)
You're right though I think, the original code clamps like this:

```cpp
static BlockAssembler::Options ClampOptions(BlockAssembler::Options options)
{
Assert(options.coinbase_max_additional_weight <= DEFAULT_BLOCK_MAX_WEIGHT);
Assert(options.coinbase_output_max_additional_sigops <= MAX_BLOCK_SIGOPS_COST);
// Limit weight to between coinbase_max_additional_weight and DEFAULT_BLOCK_MAX_WEIGHT for sanity:
// Coinbase (reserved) outputs can safely exceed -blockmaxweight, but
...
💬 Sjors commented on pull request "mining: bugfix: Fix duplicate coinbase tx weight reservation":
(https://github.com/bitcoin/bitcoin/pull/31384#discussion_r1938239821)
We need to clarify this:

> was `3,996,000`,

to

> was `3,996,000` and any higher value was silently clamped to that.
💬 Sjors commented on pull request "mining: bugfix: Fix duplicate coinbase tx weight reservation":
(https://github.com/bitcoin/bitcoin/pull/31384#discussion_r1938240400)
> can lower `-blockreservedweight` to `4,000` to maintain the previous behaviour.

to

> should be aware that this value previously had no effect, since it was clamped to 3,996,000. Be careful about lowing `-blockreservedweight` to `4,000`.
💬 fanquake commented on pull request "depends: Override default build type for `libevent`":
(https://github.com/bitcoin/bitcoin/pull/31661#discussion_r1938240587)
This did break oss-fuzz. See #31770.
📝 JoeyVee8666 opened a pull request: "doing whats needed."
(https://github.com/bitcoin/bitcoin/pull/31776)
Cant Automatically merge.<!-
*** 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
...
fanquake closed a pull request: "doing whats needed."
(https://github.com/bitcoin/bitcoin/pull/31776)
📝 fanquake locked a pull request: "doing whats needed."
(https://github.com/bitcoin/bitcoin/pull/31776)
Cant Automatically merge.<!-
*** 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
...
💬 pinheadmz commented on pull request "mining: bugfix: Fix duplicate coinbase tx weight reservation":
(https://github.com/bitcoin/bitcoin/pull/31384#discussion_r1938258005)
Then perhaps something like "Reserve space for the fixed-size block header plus the largest coinbase transaction the mining software may add to the block"
💬 TheCharlatan commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#issuecomment-2628948969)
Updated fb6e0deee18dff38219c9646d2461a40de03ed66 -> f926d7ef34773b7836e94491a16021288c125b11 ([kernelApi_20](https://github.com/TheCharlatan/bitcoin/tree/kernelApi_20) -> [kernelApi_21](https://github.com/TheCharlatan/bitcoin/tree/kernelApi_21), [compare](https://github.com/TheCharlatan/bitcoin/compare/kernelApi_20..kernelApi_21))

* Added symbol exports where appropriate in the header to enable windows support
* Completely replaced the existing bitcoin-chainstate with the new kernel-API-only
...