Bitcoin Core Github
44 subscribers
121K links
Download Telegram
🚀 achow101 merged a pull request: "wallet: guard against dangling to-be-reverted db transactions"
(https://github.com/bitcoin/bitcoin/pull/29253)
💬 achow101 commented on pull request "net: enable v2transport by default":
(https://github.com/bitcoin/bitcoin/pull/29347#issuecomment-1919891645)
ACK 0bef1042ce6c459acb1de965cbccd98867a417f1
📝 mzumsande converted_to_draft a pull request: "test: use v2 everywhere for P2PConnection if --v2transport is enabled"
(https://github.com/bitcoin/bitcoin/pull/29358)
#24748 added v2 transport to the python `P2PConnection`, but so far each test that wants to make use of it needs to enable it on an individual basis.
This PR changes it so that if the test suite is run with `--v2transport` option, v2 is used in each test by default, not only for connections between two bitcoind instances as before, but also wherever `P2PConnection` is used. Individual tests can override this global option.

To do that, a few tests need to be adjusted, which is done in the fir
...
🚀 achow101 merged a pull request: "net: enable v2transport by default"
(https://github.com/bitcoin/bitcoin/pull/29347)
👍 ryanofsky approved a pull request: "wallet: Fix migration of blank wallets"
(https://github.com/bitcoin/bitcoin/pull/28976#pullrequestreview-1854779593)
Code review ACK c11c404281d2d0e22967e30e16c0733db84f4eee
💬 ryanofsky commented on pull request "wallet: Fix migration of blank wallets":
(https://github.com/bitcoin/bitcoin/pull/28976#discussion_r1473429116)
In commit "wallet: Skip key and script migration for blank wallets" (8c127ff1edb6b9a607bf1ad247893347252a38e3)

> I'm pretty sure that it was unset whenever something was imported.

This should be true but it would be good to raise an error if the blank flag is set and these records do exist. Seems like it would easy to check for, given the HasLegacyRecords() function. It would document the assumption being made by this code, and provide better error checking.
💬 TheCharlatan commented on pull request "kernel: Remove dependency on CScheduler":
(https://github.com/bitcoin/bitcoin/pull/28960#issuecomment-1919923411)
Rebased cb47c334e0b3d654b5b56ae762aa2a2c33ae2743 -> 6752d218caeed1111f2520130c156b9ef42ba805 ([noGlobalSignals_12](https://github.com/TheCharlatan/bitcoin/tree/noGlobalSignals_12) -> [noGlobalSignals_13](https://github.com/TheCharlatan/bitcoin/tree/noGlobalSignals_13), [compare](https://github.com/TheCharlatan/bitcoin/compare/noGlobalSignals_12..noGlobalSignals_13))

* Fixed silent merge conflict with https://github.com/bitcoin/bitcoin/pull/28170
💬 achow101 commented on pull request "Nuke adjusted time from validation (attempt 2)":
(https://github.com/bitcoin/bitcoin/pull/28956#issuecomment-1919931046)
ACK ff9039f6ea876bab2c40a06a93e0dd087f445fa2
🚀 achow101 merged a pull request: "Nuke adjusted time from validation (attempt 2)"
(https://github.com/bitcoin/bitcoin/pull/28956)
🤔 TheCharlatan reviewed a pull request: "Nuke adjusted time from validation (attempt 2)"
(https://github.com/bitcoin/bitcoin/pull/28956#pullrequestreview-1854827936)
ACK ff9039f6ea876bab2c40a06a93e0dd087f445fa2
💬 mzumsande commented on pull request "test: use v2 everywhere for P2PConnection if --v2transport is enabled":
(https://github.com/bitcoin/bitcoin/pull/29358#issuecomment-1919948720)
Hmm, "test each commit fails" The problem is that commit 606f4f32014f029a6999d7f94b7231fefafdf55f (which switches `P2PConnection` to v2 by default) is enabled last. It might be best to squash everything into one big commit (even if that might make review a bit harder) or I'd have to add some temporary workarounds. Opinions?
🚀 ryanofsky merged a pull request: "wallet: Fix migration of blank wallets"
(https://github.com/bitcoin/bitcoin/pull/28976)
🤔 murchandamus reviewed a pull request: "wallet: Fix migration of blank wallets"
(https://github.com/bitcoin/bitcoin/pull/28976#pullrequestreview-1854840359)
Post merge ACK
🚀 achow101 merged a pull request: "init: settings, do not load auto-generated warning msg"
(https://github.com/bitcoin/bitcoin/pull/29301)
💬 achow101 commented on pull request "test: fix intermittent failure in p2p_v2_earlykeyresponse":
(https://github.com/bitcoin/bitcoin/pull/29352#issuecomment-1920002096)
ACK 9642aefb81a9c87227eccf9997380024247ed1fc
💬 achow101 commented on pull request "fuzz: extend ConsumeNetAddr() to return I2P and CJDNS addresses":
(https://github.com/bitcoin/bitcoin/pull/26859#issuecomment-1920010176)
ACK b851c5385d0a0acec4493be1561cea285065d5dc
🚀 achow101 merged a pull request: "test: fix intermittent failure in p2p_v2_earlykeyresponse"
(https://github.com/bitcoin/bitcoin/pull/29352)
💬 hernanmarino commented on pull request "assumeutxo, rpc: Add 'start' parameter to loadtxoutset":
(https://github.com/bitcoin/bitcoin/pull/28659#issuecomment-1920019109)
> Ok, maybe turn it into draft, until you are done and the CI is passing?

Actually, I 'd like to ask you and @pablomartin4btc (who also reviewed) a question . I am convinced now to follow @ryanofsky ( and @Sjors ) suggestion of implementing this with 3 commands instead of the current approach of subcommands.

If you agree with this new approach , I think I'll close this PR and implement the functionality in one (or two) followup PRs implementing the new commands, since there is no point in
...
💬 furszy commented on pull request "wallet: guard against dangling to-be-reverted db transactions":
(https://github.com/bitcoin/bitcoin/pull/29253#discussion_r1473503275)
ok noted, thanks!
🚀 achow101 merged a pull request: "fuzz: extend ConsumeNetAddr() to return I2P and CJDNS addresses"
(https://github.com/bitcoin/bitcoin/pull/26859)