💬 glozow commented on issue "Package Relay Project Tracking":
(https://github.com/bitcoin/bitcoin/issues/27463#issuecomment-1573736784)
> If the state-of-art implementation of BIP331 can be linked here or in the BIP (Like there is https://github.com/glozow/bitcoin/pull/8 and https://github.com/glozow/bitcoin/pull/9) ?
> From a reviewer perspective, ideally it’s better to have the proposed specification changes, the proposed Core code changes and the backend code of broadcaster client (e.g Lightning) to grasp a correct mental model of all the interactions.

Forgot to respond to this earlier - #27742 is the branch you're lookin
...

💬 instagibbs commented on issue "-fallbackfee should apply to estimatesmartfee":
(https://github.com/bitcoin/bitcoin/issues/27415#issuecomment-1573751777)
Any long-running node has them if the node has ever restarted. I just grabbed one locally when needed.

💬 instagibbs commented on issue "-fallbackfee should apply to estimatesmartfee":
(https://github.com/bitcoin/bitcoin/issues/27415#issuecomment-1573767151)
https://github.com/bitcoin/bitcoin/pull/27622 might want to keep an eye on this though, I think we should probably have a flag allowing any age of file, exposed on regtest/hidden only?

👍 furszy approved a pull request: "walletdb: Add PrefixCursor"
(https://github.com/bitcoin/bitcoin/pull/27790#pullrequestreview-1457533319)
ACK ba616b93

Nice `db_cursor_prefix_byte_test` test case 👌🏼.

💬 TheCharlatan commented on pull request "kernel: Remove shutdown from kernel library":
(https://github.com/bitcoin/bitcoin/pull/27711#discussion_r1214417257)
Thank you for your comprehensive answer. I think it boils down to: A fatal error should not necessarily have to trigger an interrupt for all kernel operations. The user of the library should decide this. A `startShutdown` should interrupt all operations in order to cleanly halt kernel execution.

> I don't think it should be necessary for the kernel to call out to the application cancel it's own execution if it can just call the cancellation object directly.

Going back to this earlier rema
...

🤔 instagibbs reviewed a pull request: "Fee estimation: avoid serving stale fee estimate"
(https://github.com/bitcoin/bitcoin/pull/27622#pullrequestreview-1457521354)
Looking good, just a couple of test suggestions.

Also wondering if we should include a hidden argument to allow older files for testing reasons, re: https://github.com/bitcoin/bitcoin/issues/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.

💬 instagibbs commented on pull request "Fee estimation: avoid serving stale fee estimate":
(https://github.com/bitcoin/bitcoin/pull/27622#discussion_r1214407758)
to make this test less order-dependent, you can attempt to delete the file, then run the test

💬 instagibbs commented on pull request "Fee estimation: avoid serving stale fee estimate":
(https://github.com/bitcoin/bitcoin/pull/27622#discussion_r1214415612)
might be good to test that it does it *every hour*, and that the output files should be matching provided we don't see blocks in the interim, also, check that what is written on node shutdown also matches?

That way we can have higher assurance on the timing of the flushing and the contents matching what we already do.

💬 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?