Bitcoin Core Github
43 subscribers
122K links
Download Telegram
pinheadmz closed an issue: "anchors.dat doesn't support V2 addresses"
(https://github.com/bitcoin/bitcoin/issues/20511)
💬 brunoerg commented on pull request "test: cover addrv2 anchors by adding TorV3 to CAddress in messages.py":
(https://github.com/bitcoin/bitcoin/pull/27452#issuecomment-1505407282)
Did you take a look at #27295?
💬 pinheadmz commented on pull request "test: cover addrv2 anchors by adding TorV3 to CAddress in messages.py":
(https://github.com/bitcoin/bitcoin/pull/27452#issuecomment-1505412445)
> Did you take a look at #27295?

I will now!
🚀 fanquake merged a pull request: "doc: update OpenBSD build docs for 7.3 (external signer support available)"
(https://github.com/bitcoin/bitcoin/pull/27449)
💬 pinheadmz commented on pull request "test: add support for all networks in `deserialize_v2`":
(https://github.com/bitcoin/bitcoin/pull/27295#discussion_r1164252589)
Is this correct for torv3? I had to add version and checksum in https://github.com/bitcoin/bitcoin/pull/27452
💬 ryanofsky commented on pull request "refactor: Extract common/args from util/system":
(https://github.com/bitcoin/bitcoin/pull/27419#issuecomment-1505447794)
I think this PR could be labeled move-only instead of refactoring, because no code is changing other than #includes.
💬 ajtowns commented on pull request "mempool: disallow txns under min relay fee, even in packages":
(https://github.com/bitcoin/bitcoin/pull/26933#issuecomment-1505483056)
I guess I don't really understand how this PR on its own helps anything -- we don't do package relay yet, so except for a regtest only rpc, these changes should be unobservable?

Would it make sense to replace this with a PR that enabled package relay without v3 or package RBF, and included this policy? I think that would be the first ~10 commits from #25038? That seems like it would be more meaningful progress...
💬 ajtowns commented on pull request "mempool / miner: regularly flush <=0-fee entries and mine everything in the mempool":
(https://github.com/bitcoin/bitcoin/pull/27018#discussion_r1164322514)
Yeah, fair point -- the claim should be "setting -blockminfee higher than zero may result in txs remaining in your mempool that will never be mined"; but as long as that's not the default behaviour, that doesn't seem particularly problematic? (TBH, I'd rather be able to pass blockminfee/blockmaxweight as getblocktemplate parameters)

Sounds like this should be closed though?
💬 ryanofsky commented on pull request "refactor: Remove CAddressBookData::destdata":
(https://github.com/bitcoin/bitcoin/pull/27224#issuecomment-1505513192)
Rebased d34323544d7283bf46bb154a90b8feca443b103e -> a5986e82dd2b8f923d60f9e31760ef62b9010105 ([`pr/nodest.19`](https://github.com/ryanofsky/bitcoin/commits/pr/nodest.19) -> [`pr/nodest.20`](https://github.com/ryanofsky/bitcoin/commits/pr/nodest.20), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/nodest.19-rebase..pr/nodest.20)) due to conflict with #27217
👍 pinheadmz approved a pull request: "Add "warnings", deprecate "warning" in {create,load,unload,restore}wallet"
(https://github.com/bitcoin/bitcoin/pull/27279#pullrequestreview-1381620202)
re-ACK 7ccdd741fe1544c13b2a9b7baa5c5727e84d6e55

confirmed trivial diff since last review, ran unit and functional tests locally, ran in regtest one more time to check UX with and without deprecated rpc flag

<details><summary>Show Signature</summary>

```
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

ACK 7ccdd741fe1544c13b2a9b7baa5c5727e84d6e55
-----BEGIN PGP SIGNATURE-----

iQIzBAEBCAAdFiEE5hdzzW4BBA4vG9eM5+KYS2KJyToFAmQ20YEACgkQ5+KYS2KJ
yTqF/Q/8DnfN9Esae47Y1bvswkY5oX93SBB59pU
...
📝 kevkevinpal opened a pull request: "test: added coverage to rpc_cantxoutset.py"
(https://github.com/bitcoin/bitcoin/pull/27453)
Included a test that checks if an invalid first argument is entered we receive a rpc error. The rpc should fail if "start", "status" or "abort" is not the first command.

Relavant: mentioned in https://github.com/bitcoin/bitcoin/pull/27422
💬 kevkevinpal commented on pull request "test: add coverage to rpc_scantxoutset.py":
(https://github.com/bitcoin/bitcoin/pull/27422#issuecomment-1505538644)
> Yes can be something like `assert_raises_rpc_error(-8, "Invalid action 'word'", self.nodes[0].scantxoutset, "word")`

opened PR for this https://github.com/bitcoin/bitcoin/pull/27453
let me know if you have any comments
💬 instagibbs commented on pull request "mempool: disallow txns under min relay fee, even in packages":
(https://github.com/bitcoin/bitcoin/pull/26933#issuecomment-1505540220)
I think either way is "meaningful progress", but I'm not averse to coupling the commits if we're in agreement on this PR's commits.
💬 pinheadmz commented on pull request "Fix logging RPC and -debugexclude with 0/none values, add test coverage, improve docs":
(https://github.com/bitcoin/bitcoin/pull/27231#discussion_r1164350707)
Well I meant something like a switch statement with `"-debug"` and `"-debugexclude"` literal strings as cases. I think that would make it more readable and if anything is ever added to `DEBUG_LOG_OPTS[]` it won't necessarily break. But that might just be too long or out of style
💬 furszy commented on pull request "wallet: finish addressbook encapsulation":
(https://github.com/bitcoin/bitcoin/pull/26836#issuecomment-1505555162)
Rebased, conflicts solved.
Plus, per https://github.com/bitcoin/bitcoin/pull/26761#discussion_r1134636537 request, added coverage for wallet address book migration process.
💬 pinheadmz commented on pull request "Fix logging RPC and -debugexclude with 0/none values, add test coverage, improve docs":
(https://github.com/bitcoin/bitcoin/pull/27231#discussion_r1164360080)
Ah I see, these cases are essentially handled by `IsNoneCategory()` 👍
💬 fanquake commented on pull request "ci: use LLVM/Clang 15 and Valgrind 3.19 in Valgrind jobs":
(https://github.com/bitcoin/bitcoin/pull/27444#discussion_r1164364129)
I'll leave this for now, and follow up in the build-from-source PR
💬 fanquake commented on pull request "ci: use LLVM/Clang 15 and Valgrind 3.19 in Valgrind jobs":
(https://github.com/bitcoin/bitcoin/pull/27444#issuecomment-1505564980)
> What is the error message on aarch64? Does it happen with gcc and clang?

I'll open a separate issue for this.

I've opened some PRs in other repos in readiness for this change:
* https://github.com/bitcoin-core/qa-assets/pull/115
* https://github.com/MarcoFalke/b-c-nightly/pull/7

So going to go-ahead and merge this.
💬 kristapsk commented on pull request "RPC: Add universal options argument to listtransactions":
(https://github.com/bitcoin/bitcoin/pull/22807#issuecomment-1505567777)
Rebased
💬 MarcoFalke commented on pull request "ci: use LLVM/Clang 15 and Valgrind 3.19 in Valgrind jobs":
(https://github.com/bitcoin/bitcoin/pull/27444#discussion_r1164373407)
Any reason to use `clang-15` over `clang`? Absent a reason, this makes it harder to bump or quickly modify the image name tag