💬 pinheadmz commented on pull request "updated":
(https://github.com/bitcoin/bitcoin/pull/27451#issuecomment-1505340415)
https://bitcoin.stackexchange.com/questions

💬 tarunraheja84 commented on pull request "updated":
(https://github.com/bitcoin/bitcoin/pull/27451#issuecomment-1505343858)
Thankyou very much

💬 jonatack commented on pull request "updated":
(https://github.com/bitcoin/bitcoin/pull/27451#issuecomment-1505352444)
> Hello, I am completely new to open source. This was my first PR. Compiling Bitcoin Core into pc is the first step for a new developer to get started with which is not mentioned in Readme anywhere.

@tarunraheja84 Here are some resources you may find useful: https://jonatack.github.io/articles

💬 stickies-v commented on pull request "Allow configuring target block time for a signet":
(https://github.com/bitcoin/bitcoin/pull/27446#issuecomment-1505365250)
> So in the end the parameter would need to be distributed the same way as `signetchallenge`

I had similar thoughts and think eventually we may want to introduce a more encompassing (and upgradeable) parameter such as `-signetconfig` in which all signet consensus parameters are encoded (currently just challenge and blocktime), but I think starting with the current modular approach is fine given that it's signet.

💬 instagibbs commented on pull request "mempool: disallow txns under min relay fee, even in packages":
(https://github.com/bitcoin/bitcoin/pull/26933#issuecomment-1505372661)
concept ACK

I find both variants unsatisfactory for different reasons, but while we're working on something potentially better, I think this small change is the most reasonable, as it can be removed simply if improved upon. Meanwhile, we can continue moving forward with package relay/RBF/et al.

💬 mzumsande commented on issue "index: ThreadSanitizer: data race on vptr ":
(https://github.com/bitcoin/bitcoin/issues/27355#issuecomment-1505373331)
Hmm, then my guess that it could be the timeout was probably wrong. Could you run the test in isolation with `-- -printtoconsole` and post the log?

💬 glozow commented on pull request "Remove BIP35 mempool p2p message":
(https://github.com/bitcoin/bitcoin/pull/27426#issuecomment-1505385773)
> > let's say we came up with a way [...] to try to sync the top of node mempools from time to time. Is there a reason that would be a bad idea
>
> I think that's a different way of describing "rebroadcast", which seems like a good idea: it improves miner revenue, improves tx propagation, improves block propagation, makes eclipse and pinning attacks harder... At least assuming it can be implemented without too many terrible tradeoffs.

+1. I don't think it implies BIP35 support is good, tho
...

📝 pinheadmz opened a pull request: "test: cover addrv2 anchors by adding TorV3 to CAddress in messages.py"
(https://github.com/bitcoin/bitcoin/pull/27452)
Adds test coverage for https://github.com/bitcoin/bitcoin/pull/20516 to ensure that https://github.com/bitcoin/bitcoin/issues/20511 is completed and may be closed.

This PR adds a test case to `feature_anchors.py` where an onion v3 address is set as a blocks-only relay peer and then shutdown, ensuring that the address is saved to anchors.dat in addrv2 format. We then ensure that bitcoin attempts to reconnect to that anchor address on restart.

To compute the addrv2 serialization of the onion
...

💬 pinheadmz commented on issue "anchors.dat doesn't support V2 addresses":
(https://github.com/bitcoin/bitcoin/issues/20511#issuecomment-1505405451)
> @pinheadmz I think so.

Confirmed with tests! See https://github.com/bitcoin/bitcoin/pull/27452

✅ 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