Bitcoin Core Github
44 subscribers
120K links
Download Telegram
⚠️ dkatzan opened an issue: "Assertion pindexPrev && pindexPrev == chainstate.m_chain.Tip() when running regtest"
(https://github.com/bitcoin/bitcoin/issues/31562)
### Is there an existing issue for this?

- [X] I have searched the existing issues

### Current behaviour

I'm running a regtest chain in my CI\CD for some tests, occasionally, I will see my bitcoind fail with an assertion
`Assertion failed: pindexPrev && pindexPrev == chainstate.m_chain.Tip() (validation.cpp: TestBlockValidity: 4338)`

### Expected behaviour

The node to be stable and not crash

### Steps to reproduce

I don't have steps to reproduce, this happens very rearily as part of our
...
💬 maflcko commented on issue "b-msghand invoked oom-killer: Master (v28.99) crashing during IBD":
(https://github.com/bitcoin/bitcoin/issues/31561#issuecomment-2561130717)
> Seems to be the same issue as #30001

I don't think they are the same. In fact, there are many differences: You are using `bitcoind` with `-txindex` and default settings, the other person was using the GUI with a large dbcache and external HDD for the blocksdir.

It looks similar to https://github.com/bitcoin/bitcoin/issues/31041, though.

Given that it is likely not heap (https://github.com/bitcoin/bitcoin/issues/31041#issuecomment-2514072312), it would be good to debug where the usage
...
💬 tdb3 commented on pull request "descriptor: remove unreachable verification for `pkh`":
(https://github.com/bitcoin/bitcoin/pull/31555#discussion_r1896760760)
It's definitely shorter, for sure. A thought here is that if another context is added and it's not appropriate for the function, the check for allowed contexts would still work (conversely the !P2WPKH check would miss it). If another context is added and it is appropriate, then we're informed to add it (the assume would fail), which also serves as a reminder to ensure the function is updated appropriately.
💬 brunoerg commented on pull request "descriptor: remove unreachable verification for `pkh`":
(https://github.com/bitcoin/bitcoin/pull/31555#discussion_r1896774367)
Yes, fair enough. I will address it.
💬 brunoerg commented on pull request "descriptor: remove unreachable verification for `pkh`":
(https://github.com/bitcoin/bitcoin/pull/31555#discussion_r1896775059)
Done.
💬 maflcko commented on issue "Assertion pindexPrev && pindexPrev == chainstate.m_chain.Tip() when running regtest":
(https://github.com/bitcoin/bitcoin/issues/31562#issuecomment-2561185322)
> I don't have steps to reproduce, this happens very rearily as part of our CI\CD that runs a simple regtest chains, mines a few blocks, makes a few transactions and managing a few wallets

Thanks for the report! I presume, you are calling the test-only `generateblock` RPC? If not, it would be good to have minimal reproducer, or a link to a log of the CI run, or the test code that hit the issue. Without a log or anything to reproduce, it will be harder to see where the bug happened.
📝 maflcko opened a pull request: "rpc: Extend scope of validation mutex in generatetoaddress"
(https://github.com/bitcoin/bitcoin/pull/31563)
The mutex (required by TestBlockValidity) must be held after creating the block, until TestBlockValidity is called. Otherwise, it is possible that the chain advances in the meantime and leads to a crash in TestBlockValidity: `Assertion failed: pindexPrev && pindexPrev == chainstate.m_chain.Tip() (validation.cpp: TestBlockValidity: 4338)`

Fixes #31562
👍 tdb3 approved a pull request: "descriptor: remove unreachable verification for `pkh`"
(https://github.com/bitcoin/bitcoin/pull/31555#pullrequestreview-2522032911)
cr ACK 366ae00b779acd59a61719422f0597acb17fb3e0
💬 dkatzan commented on issue "Assertion pindexPrev && pindexPrev == chainstate.m_chain.Tip() when running regtest":
(https://github.com/bitcoin/bitcoin/issues/31562#issuecomment-2561216463)
Yep, I'm calling `generateblock` multiple times
my tests are also running concurrently, so it might be called concurrently

here are the latest logs I have from before the crash
[extract-2024-12-24T14_43_27.491Z.csv](https://github.com/user-attachments/files/18240488/extract-2024-12-24T14_43_27.491Z.csv)
💬 maflcko commented on pull request "qa: Limit `-maxconnections` in tests":
(https://github.com/bitcoin/bitcoin/pull/31537#issuecomment-2561217764)
re-ACK d9d5bc2e7466033d989432f53a112325fa3d6d4a

only change is a whitespace fix
💬 maflcko commented on issue "Assertion pindexPrev && pindexPrev == chainstate.m_chain.Tip() when running regtest":
(https://github.com/bitcoin/bitcoin/issues/31562#issuecomment-2561221830)
Thx, #https://github.com/bitcoin/bitcoin/pull/31563 should fix the assert crash
💬 dkatzan commented on issue "Assertion pindexPrev && pindexPrev == chainstate.m_chain.Tip() when running regtest":
(https://github.com/bitcoin/bitcoin/issues/31562#issuecomment-2561232867)
Thx alot!
so is this expected to reach the next release cycle? which version will it be included in?
💬 ismaelsadeeq commented on issue "Assertion pindexPrev && pindexPrev == chainstate.m_chain.Tip() when running regtest":
(https://github.com/bitcoin/bitcoin/issues/31562#issuecomment-2561262673)
> so is this expected to reach the next release cycle? which version will it be included in?

After it is reviewed and merged yes it will be available in the next release.
You can check #31029 for the plans regarding the next release.
You can also volunteer to test and ensure that #31563 addresses your issue.
🤔 Prabhat1308 reviewed a pull request: "fuzz: Abort if system time is called without mock time being set"
(https://github.com/bitcoin/bitcoin/pull/31549#pullrequestreview-2522097613)
Concept ACK .

Left a comment for clarification .
💬 Prabhat1308 commented on pull request "fuzz: Abort if system time is called without mock time being set":
(https://github.com/bitcoin/bitcoin/pull/31549#discussion_r1896853488)
Why Would I need to know what mocktime have been set by the fuzzer in a general situation?
💬 maflcko commented on issue "Intermittent issue in feature_index_prune.py: assert_equal(pruneheight_new, 248) AssertionError: not(249 == 248)":
(https://github.com/bitcoin/bitcoin/issues/31446#issuecomment-2561273816)
https://cirrus-ci.com/task/6171541521563648?logs=ci#L2939
📝 Sjors opened a pull request: "Add checkblock RPC and checkBlock() to Mining interface"
(https://github.com/bitcoin/bitcoin/pull/31564)
## Rationale

### Verifying block templates (no PoW)

Stratum v2 and DATUM allow miners to generate their own block template. Pools may wish (or need) to verify these templates. This typically involves comparing mempools, asking miners to providing missing transactions and then reconstructing the proposed block.[^0] This is not sufficient to ensure a proposed block is actually valid. In some schemes miners could take advantage of incomplete validation[^1].

The Stratum Reference Implementa
...
💬 Sjors commented on pull request "Add checkblock RPC and checkBlock() to Mining interface":
(https://github.com/bitcoin/bitcoin/pull/31564#issuecomment-2561298038)
I wrote on IRC:

>> Does anyone know what `UpdateUncommittedBlockStructures` is good for in the `submitblock` RPC?

To which @sipa responded:

> Sjors[m]: it looks like it adds the "witness nonce" if missing (the witness for the coinbase input is a reserved field for adding future commitments, but it must be present, so that function adds a 0)

This PR omits `UpdateUncommittedBlockStructures`. It would be nice to come up with a test that shows what it does and fails on the current code.
🤔 tdb3 reviewed a pull request: "rpc: allow writing UTXO set to a named pipe, introduce dump_to_sqlite.sh script"
(https://github.com/bitcoin/bitcoin/pull/31560#pullrequestreview-2522042088)
Approach ACK

Great feature!

Did some manual santiy testing on mainnet:
- Used `dumptxoutset` to create a dump (with a node synced to block 876,186), `utxo_to_sqlite.py` to covert to a sqlite file, and opened/parsed in python. Conversion seemed successful, the correct number of coins were present in the table
- Used `dump_to_sqlite.sh` to do the same with fifo (but with a node synced to block 200,000), then open/parsed in python. Conversion seemed successful, the correct number of coins
...
💬 tdb3 commented on pull request "rpc: allow writing UTXO set to a named pipe, introduce dump_to_sqlite.sh script":
(https://github.com/bitcoin/bitcoin/pull/31560#discussion_r1896861260)
Rather than have this wait indefinitely, might be better to specify a `timeout` (e.g. `CONVERSION_TIMEOUT = 60`, `p.wait(timeout=CONVERSION_TIMEOUT)`). This could allow earlier failure detection (i.e. instead of relying on the longer CI timeout).
💬 tdb3 commented on pull request "rpc: allow writing UTXO set to a named pipe, introduce dump_to_sqlite.sh script":
(https://github.com/bitcoin/bitcoin/pull/31560#discussion_r1896865020)
`2024-present`