💬 jarolrod commented on pull request "rpc: Be able to access RPC parameters by name":
(https://github.com/bitcoin/bitcoin/pull/27788#issuecomment-1573793819)
Concept ACK

💬 glozow commented on pull request "mempool / rpc: add getprioritisedtransactions, delete a mapDeltas entry when delta==0":
(https://github.com/bitcoin/bitcoin/pull/27501#discussion_r1214440049)
Correct, it is not used. I added it to address this comment: https://github.com/bitcoin/bitcoin/pull/27501#discussion_r1184516693.

💬 glozow commented on issue "-fallbackfee should apply to estimatesmartfee":
(https://github.com/bitcoin/bitcoin/issues/27415#issuecomment-1573838410)
I'm going to close this particular request as I think it's clear we shouldn't add a fallback feerate for the estimator.
Will keep in mind the importance of being able to easily load a fee_estimates.dat for testing. Thanks very much for your feedback @kylezs!

✅ glozow closed an issue: "-fallbackfee should apply to estimatesmartfee"
(https://github.com/bitcoin/bitcoin/issues/27415)

💬 kylezs commented on issue "-fallbackfee should apply to estimatesmartfee":
(https://github.com/bitcoin/bitcoin/issues/27415#issuecomment-1573843901)
Sounds good. Cheers!

💬 glozow commented on pull request "Fee estimation: avoid serving stale fee estimate":
(https://github.com/bitcoin/bitcoin/pull/27622#issuecomment-1573845849)
> Also wondering if we should include a hidden argument to allow older files for testing reasons, re: #27415
>
> I've used it for systems I've built for integration testing, and not having to touch a file during testing to refresh it would be a simplification.
>
> > For testing, they could mock the time right?
>
> Depends on your test setup, really. It might interfere during integration tests which require a realistic clock?

Makes sense, seems fine as a regtest-only option for testin
...

🤔 instagibbs reviewed a pull request: "[NO MERGE] BIP331 Ancestor Package Relay"
(https://github.com/bitcoin/bitcoin/pull/27742#pullrequestreview-1451533873)
Along with the gist, focused on orphan protection and bucketing for concept understanding. Dumping comments related to those, and left a note on the gist.

💬 instagibbs commented on pull request "[NO MERGE] BIP331 Ancestor Package Relay":
(https://github.com/bitcoin/bitcoin/pull/27742#discussion_r1210641158)
or what?

💬 instagibbs commented on pull request "[NO MERGE] BIP331 Ancestor Package Relay":
(https://github.com/bitcoin/bitcoin/pull/27742#discussion_r1210682865)
[[txorphange] use wtxid instead of txid, handle nullptr](https://github.com/bitcoin/bitcoin/pull/27742/commits/22e93f08a50a206b54717b07682e73a751cc6755)

maybe change commit message to be clearer we're erasing by wtxid, as the primary change?

💬 instagibbs commented on pull request "[NO MERGE] BIP331 Ancestor Package Relay":
(https://github.com/bitcoin/bitcoin/pull/27742#discussion_r1210707099)
wouldn't reference it in case it drifts? If it's exactly the same and always should be, just use this constant value?

💬 instagibbs commented on pull request "[NO MERGE] BIP331 Ancestor Package Relay":
(https://github.com/bitcoin/bitcoin/pull/27742#discussion_r1210724340)
really close to `OrphanageAddTx` even though the comments are clear of its purpose here. Maybe `AddKnownOrphanTx` or `AddAlreadyKnownOrphanTx`?

💬 instagibbs commented on pull request "[NO MERGE] BIP331 Ancestor Package Relay":
(https://github.com/bitcoin/bitcoin/pull/27742#discussion_r1210708748)
> is_wtxid

where is this defined?

💬 instagibbs commented on pull request "[NO MERGE] BIP331 Ancestor Package Relay":
(https://github.com/bitcoin/bitcoin/pull/27742#discussion_r1210729694)
`is_wtxid=false`?

💬 instagibbs commented on pull request "[NO MERGE] BIP331 Ancestor Package Relay":
(https://github.com/bitcoin/bitcoin/pull/27742#discussion_r1210769183)
where did this behavior live prior, where this isn't a behavior change?

💬 instagibbs commented on pull request "[NO MERGE] BIP331 Ancestor Package Relay":
(https://github.com/bitcoin/bitcoin/pull/27742#discussion_r1210736698)
What does this last sentence mean?

💬 instagibbs commented on pull request "[NO MERGE] BIP331 Ancestor Package Relay":
(https://github.com/bitcoin/bitcoin/pull/27742#discussion_r1211763570)
since we're touching this, would you mind annotating the bool as `/* best */` to make it clearer what this is doing? New to tx tracking; would be helpful

💬 petertodd commented on pull request "Ignore datacarrier limits for dataless OP_RETURN outputs":
(https://github.com/bitcoin/bitcoin/pull/27261#issuecomment-1573858529)
> Would you mind submitting the test to test current `master` behavior for `-nodatacarrier`, or would you mind if someone else did that?

I certainly don't mind if someone else does that.

💬 pinheadmz commented on issue "Add test vectors in BIP143 into tx_valid.json / sighash.json":
(https://github.com/bitcoin/bitcoin/issues/14308#issuecomment-1573869228)
I think it's safe to close this as won't-fix for the reasons stated above. Pull requests are always welcome!

✅ pinheadmz closed an issue: "Add test vectors in BIP143 into tx_valid.json / sighash.json"
(https://github.com/bitcoin/bitcoin/issues/14308)

💬 glozow commented on pull request "[NO MERGE] BIP331 Ancestor Package Relay":
(https://github.com/bitcoin/bitcoin/pull/27742#discussion_r1214484568)
> Should this function ever be called with a protected orphan?

Yes. It's called when orphans expire as well.

> Tests don't cover these lines for protected peers

Sounds like I neglected to write a test for this. Expiry is pretty long, but theoretically we can hit this if we have multiple peers stalling us on the resolution of this orphan. We might happen to be trying package relay with one of the peers when the expiration is reached.