Bitcoin Core Github
43 subscribers
122K links
Download Telegram
🤔 glozow reviewed a pull request: "Refactor: MiniWallet functionality for more readable code in functional test `rpc_signtransactionwithkey.py`"
(https://github.com/bitcoin/bitcoin/pull/30667#pullrequestreview-2254476437)
I'm going to close this pull request for now. If you'd like to work on the issue, here are some hints before you open a new pull request.

1. Please ensure your code works before submitting a pull request. There is a lot of documentation for the functional test suite if you need help https://github.com/bitcoin/bitcoin/blob/master/test/functional/README.md
2. Write code a bit more carefully. `node` isn't an attribute; you forgot an `s`. `MiniWallet` does not have a `sendrawtransaction` functio
...
💬 glozow commented on pull request "Refactor: MiniWallet functionality for more readable code in functional test `rpc_signtransactionwithkey.py`":
(https://github.com/bitcoin/bitcoin/pull/30667#discussion_r1727027744)
Ah apologies, I was too hasty and didn't notice that this is for the funding transactions. You can ignore that.
glozow closed a pull request: "Refactor: MiniWallet functionality for more readable code in functional test `rpc_signtransactionwithkey.py`"
(https://github.com/bitcoin/bitcoin/pull/30667)
💬 instagibbs commented on pull request "refactor: TxDownloadManager + fuzzing":
(https://github.com/bitcoin/bitcoin/pull/30110#issuecomment-2304734471)
> Doesn't look like ReceivedTx ever returns a package to validate in any of the fuzz tests (500 CPU hours each):

At a minimum `TESTED_TX_RESULTS` is missing `TxValidationResult::TX_RECONSIDERABLE`, so nothing is ever failing for the right reason to try a package.

I restricted the set of possible transactions down to just a parent/child combo in TRANSACTIONS and was able to trigger it quickly, so it's likely reachable with that one change.
💬 TheCharlatan commented on pull request "Introduce waitTipChanged() mining interface, replace RPCNotifyBlockChange and drop CRPCSignals":
(https://github.com/bitcoin/bitcoin/pull/30409#discussion_r1727105521)
In commit b4d37fa70506129ba4a4d218bc49ce8ca9d8380c

I think there is a significant change in behaviour here between `waitTipChanged`, or rather `g_best_block`, and the rpc notification through `blockTip`. AFAICT the `blockTip` notification reports on completed operations in `ActivateBestChain` and `InvalidatedBlock`, while `g_best_block` is updated through the lower-level `ConnectTip` and `DisconnectTip`, which may respectively be called iteratively by `ActivateBestChain` and `InvalidateBlock`
...
💬 hebasto commented on pull request "build: Mark `x86_64-linux-gnu` release binaries as CET-enabled":
(https://github.com/bitcoin/bitcoin/pull/30685#issuecomment-2304745347)
The PR description has been updated with a runtime check example.
🤔 stickies-v reviewed a pull request: "[27.x] Even more backports"
(https://github.com/bitcoin/bitcoin/pull/30558#pullrequestreview-2254624031)
code review ACK https://github.com/bitcoin/bitcoin/commit/885c4b7bc65c294b4b2fac91392d23a37a1f6485 but 8932090f9ecd933c86ee7800c0858e8d1c0bd40b is missing backport metadata in the commit message.

I verified that all backport commits are clean, except:

https://github.com/bitcoin/bitcoin/commit/8932090f9ecd933c86ee7800c0858e8d1c0bd40b backported from https://github.com/bitcoin/bitcoin/commit/590456e3f1043ba0680e0afec9fd7653db1098bb: merge conflicts from 539404fe0f and 8950053636 which have b
...
🚀 glozow merged a pull request: "Fix maybe-uninitialized warning in IsSpentKey"
(https://github.com/bitcoin/bitcoin/pull/30691)
👍 ryanofsky approved a pull request: "test: [refactor] Use m_rng directly"
(https://github.com/bitcoin/bitcoin/pull/30571#pullrequestreview-2254625428)
Code review ACK 38b03bc3af9fbf483b5a29a54a5ffb36c67b6b41. Just another random seed bug in the prevector tests fixed since last review!
💬 ryanofsky commented on pull request "test: [refactor] Use m_rng directly":
(https://github.com/bitcoin/bitcoin/pull/30571#discussion_r1727126557)
re: https://github.com/bitcoin/bitcoin/pull/30571#discussion_r1725758066

> Starting with commit https://github.com/bitcoin/bitcoin/commit/fae43a97ca947cd0802392e9bb86d9d0572c0fba, the seed is static const per process

Wow, I didn't notice `ctx_seed` was statically caching a random value before this so I've been misunderstanding what the `SeedRandomForTest` function has been doing the whole time. I think this could be made clearer by:

- Renaming `SeedRand::SEED` to `SeedRand::FIXED_SEED`
...
📝 virtu opened a pull request: "WIP: seeds: Add additional seed source and bump uptime requirements for Onion and I2P nodes"
(https://github.com/bitcoin/bitcoin/pull/30695)
This builds on #30008 and adds data [exported](https://github.com/virtu/seed-exporter) by [my crawler](https://github.com/virtu/p2p-crawler) an additional source for seed nodes. Data covers all supported network types.

### To dos
- [ ] Remove manual nodes
- [ ] Remove WIP label and rebase once #30008 gets merged

### Motivation
- Further decentralizes the seed node selection process (in the long term potentially enabling an _n_-source threshold for nodes to prevent a single source from e
...
🤔 glozow reviewed a pull request: "kernel: pre-28.x chainparams and headerssync update"
(https://github.com/bitcoin/bitcoin/pull/30658#pullrequestreview-2254691819)
ACK 221809b81cfcecb04050915eebacffda2599da42

Checked chain params against my nodes (all except testnet3, I just sanity checked that against a block explorer). Checked I get the same headerssync params.
💬 alfonsoromanz commented on pull request "test: assumeutxo: add missing tests in wallet_assumeutxo.py":
(https://github.com/bitcoin/bitcoin/pull/30455#discussion_r1727193200)
Fixed
⚠️ hodlinator opened an issue: "Unit test failures when using multiple jobs and RANDOM_CTX_SEED"
(https://github.com/bitcoin/bitcoin/issues/30696)
### Is there an existing issue for this?

- [X] I have searched the existing issues

### Current behaviour

While testing #30571 using:
```
RANDOM_CTX_SEED=1 make -j16 check
```
...I discovered that 97e16f57042cab07e5e73f6bed19feec2006e4f7 from #29625 leads to test failures within the first 10 seconds in **blockfilter_index_tests/blockfilter_index_initial_sync**, **argsman_tests/util_datadir** or **addrman_tests** (probably more).

Running without the environment variable, or single-thread
...
💬 alfonsoromanz commented on pull request "test: assumeutxo: add missing tests in wallet_assumeutxo.py":
(https://github.com/bitcoin/bitcoin/pull/30455#discussion_r1727194205)
Done. Thanks!
💬 alfonsoromanz commented on pull request "test: assumeutxo: add missing tests in wallet_assumeutxo.py":
(https://github.com/bitcoin/bitcoin/pull/30455#discussion_r1727195499)
Fixed
💬 alfonsoromanz commented on pull request "test: assumeutxo: add missing tests in wallet_assumeutxo.py":
(https://github.com/bitcoin/bitcoin/pull/30455#discussion_r1727195864)
Done
💬 achow101 commented on pull request "[WIP] seeds: Add additional seed source and bump uptime requirements for Onion and I2P nodes":
(https://github.com/bitcoin/bitcoin/pull/30695#discussion_r1727198083)
This will overwrite instead of append.
💬 alfonsoromanz commented on pull request "test: assumeutxo: add missing tests in wallet_assumeutxo.py":
(https://github.com/bitcoin/bitcoin/pull/30455#discussion_r1727198919)
Done
💬 alfonsoromanz commented on pull request "test: assumeutxo: add missing tests in wallet_assumeutxo.py":
(https://github.com/bitcoin/bitcoin/pull/30455#discussion_r1727199409)
Done. Thanks!