Bitcoin Core Github
45 subscribers
118K links
Download Telegram
💬 hebasto commented on pull request "ci: build multiprocess on most jobs":
(https://github.com/bitcoin/bitcoin/pull/30975#issuecomment-2628639378)
> But I am surprised to see clang-tidy build succeeding.

So am I.

> There were so much other clang-tidy output in the previous failing job aside from the missing input file errors fixed by the cmake change: https://cirrus-ci.com/task/4607289014878208?logs=ci#L7174. But I guess all the other new clang-tidy output was warnings not errors, so they don't cause the job to fail.

It could be the result of the absence of `WarningsAsErrors: '*'` in `src/ipc/libmultiprocess/.clang-tidy`.

> Sti
...
💬 brunoerg commented on pull request "rpc: collect transaction fees on generateblock":
(https://github.com/bitcoin/bitcoin/pull/31775#discussion_r1938131196)
```
19:24:04.713] /ci_container_base/src/node/miner.cpp:150:9: error: 'exclusive_locks_required' attribute cannot be applied to a statement
[19:24:04.713] 150 | EXCLUSIVE_LOCKS_REQUIRED(m_mempool->cs);
[19:24:04.713] | ^ ~
[19:24:04.713] /ci_container_base/src/threadsafety.h:31:54: note: expanded from macro 'EXCLUSIVE_LOCKS_REQUIRED'
[19:24:04.713] 31 | #define EXCLUSIVE_LOCKS_REQUIRED(...) __attribute__((exclusive_locks_requir
...
📝 tdb3 converted_to_draft a pull request: "rpc: add `relevant_blocks` to `scanblocks status`"
(https://github.com/bitcoin/bitcoin/pull/30713)
Currently, after starting a scan with `scanblocks` the user must wait until the scan is complete to see the associated/relevant block hashes.
This updates the `scanblocks status` result object to provide the `relevant_blocks` found so far.
This enables earlier subsequent lookup within matching blocks (e.g. to run `getdescriptoractivity` in PR #30708)

Below is an example tested on signet by starting and obtaining status of a scan for an address that has received many coinbase outputs (so the
...
💬 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)