Bitcoin Core Github
44 subscribers
121K links
Download Telegram
πŸ’¬ rkrux commented on pull request "descriptors: inference process, do not return unparsable multisig descriptors":
(https://github.com/bitcoin/bitcoin/pull/31404#discussion_r1896688596)
+1 for this doc change.
πŸ’¬ vasild commented on pull request "ci: detect outbound internet traffic generated while running tests":
(https://github.com/bitcoin/bitcoin/pull/31349#discussion_r1896697274)
Done! Much better.
πŸ’¬ vasild commented on pull request "ci: detect outbound internet traffic generated while running tests":
(https://github.com/bitcoin/bitcoin/pull/31349#discussion_r1896713349)
The table in the OP describes each task and its status wrt to this. There are 18 tasks. It is supposed to work on the top 5+4=9 tasks. The other 9 tasks are not expected to run this - e.g. Windows or tidy that does not run any tests or macOS that runs without docker and has no enough permissions.

The intention here is to have at least one task that runs this. But it will not hurt to enforce it on more. Do you want to add `CI_FAIL_IF_NO_TCPDUMP_FILE=1` to more tasks? Or reverse the logic to `C
...
πŸ’¬ brunoerg commented on pull request "descriptor: remove unreachable verification for `pkh`":
(https://github.com/bitcoin/bitcoin/pull/31555#discussion_r1896714825)
I'm not sure about this. I think `Assume(ctx != ParseScriptContext::P2WPKH)` is shorter and easier to understand.
πŸ’¬ maflcko commented on pull request "ci: detect outbound internet traffic generated while running tests":
(https://github.com/bitcoin/bitcoin/pull/31349#discussion_r1896719730)
> The other 9 tasks are not expected to run this - e.g. Windows or tidy that does not run any tests or macOS that runs without docker and has no enough permissions.

If no tests are run, then no exclusion is needed as well. Though, if this is not expected to run on tidy, I wonder why `traffic_monitor_begin "tidy"` exists? This leaves macOS, which can be excluded. The other tasks, which do not use the CI infra anyway, so don't need to be excluded as well.
πŸ’¬ ytrezq commented on issue "Please provide 32 bits builds again":
(https://github.com/bitcoin/bitcoin/issues/31557#issuecomment-2561085564)
@maflcko except bitcoin 0.17 can’t read the `wallet.dat` file I created with a more recent version of Bitcoin back when I had working 64 bits computer.

**An alternative would be to support 32 bits builds until an import solution come for android apps**.
⚠️ pinheadmz opened an issue: "b-msghand invoked oom-killer: Master (v28.99) crashing during IBD"
(https://github.com/bitcoin/bitcoin/issues/31561)
Running master (specifically 318b2a2f90dfd8ae20beca58e55e6d240a7f3d27 which is HEAD of #31415) on Ubuntu 24.10 on a digital ocean droplet with 8GB RAM. Crashing regularly during IBD. Logs always end like this:

```

2024-12-23T20:39:07Z [validation] Enqueuing BlockConnected: block hash=000000000000000000008364a7396ba08c5ab141d10136e3ad0d9fa37899d7b0 block height=814565
2024-12-23T20:39:07Z [validation] Enqueuing UpdatedBlockTip: new block hash=000000000000000000008364a7396ba08c5ab141d10136e
...
πŸ’¬ i-am-yuvi commented on pull request "test: Avoid intermittent error in assert_equal(pruneheight_new, 248)":
(https://github.com/bitcoin/bitcoin/pull/31468#issuecomment-2561108332)
> > I was thinking of using a batch syncing process instead of one-by-one to be memory efficient, but it ended up being a lot slower. Anyway, nice work, @maflcko.
>
> For fun, I had written another async alternative that reads the next block while the current block is submitted, which was faster than this pull by a few seconds, but I think trivial code is more important in tests than the fastest solution.

Interesting!! I Would love to see that as well. Can you share that!!?
πŸ’¬ maflcko commented on issue "Please provide 32 bits builds again":
(https://github.com/bitcoin/bitcoin/issues/31557#issuecomment-2561109069)
> I created with a more recent version of Bitcoin back when I had working 64 bits computer.

In theory, one could try to revive a `i686-w64-mingw32` cross-build, but the effort to get the build working and then subsequently test that the build worked feels way more expensive and non-trivial for a one-off workaround than just going with a machine for which builds are provided.

Installing a 32-bit system Linux and compiling yourself should be easier, but I haven't tried it.
πŸ’¬ maflcko commented on pull request "test: Avoid intermittent error in assert_equal(pruneheight_new, 248)":
(https://github.com/bitcoin/bitcoin/pull/31468#issuecomment-2561113673)


<details><summary>I dropped the commit, but it looked something like:</summary>


```diff
with concurrent.futures.ThreadPoolExecutor(max_workers=self.num_nodes) as rpc_threads:
- for i in range(height_from, to_height + 1):
- b = node_from.getblock(blockhash=node_from.getblockhash(i), verbosity=0)
- list(rpc_threads.map(lambda n: n.submitblock(b), self.nodes))
+ fut = rpc_threads.submit(lambda:node_from.getblock(blockhash=node_
...
⚠️ 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