💬 TheCharlatan commented on pull request "guix: Clean up manifest":
(https://github.com/bitcoin/bitcoin/pull/27811#issuecomment-1578679709)
Guix build:
```
find guix-build-$(git rev-parse --short=12 HEAD)/output/ -type f -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum
c4f2c29c7b5e1ab211ea38358517cf6c2a1e154e5d2c0d036e7750a8f5f5555a guix-build-77414d50d6f7/output/aarch64-linux-gnu/SHA256SUMS.part
c902944cce64fe7f2cee1242f18b35dbe1c5ce515e921c08c9bfebb34d0c28ca guix-build-77414d50d6f7/output/aarch64-linux-gnu/bitcoin-77414d50d6f7-aarch64-linux-gnu-debug.tar.gz
3aad68ed102f2f1a18217b3d2b2b32ba51f266c03b971f05f8bebd160bbf7d7
...

💬 vasild commented on pull request "test: add end-to-end tests for CConnman and PeerManager":
(https://github.com/bitcoin/bitcoin/pull/26812#discussion_r1219560287)
Are you suggesting to replace it with this:

```cpp
const std::string random_message_type{fuzzed_data_provider.ConsumeBytesAsString(CMessageHeader::COMMAND_SIZE)};
if (!LIMIT_TO_MESSAGE_TYPE.empty() && random_message_type != LIMIT_TO_MESSAGE_TYPE) {
continue;
}
```

💬 vasild commented on pull request "test: add end-to-end tests for CConnman and PeerManager":
(https://github.com/bitcoin/bitcoin/pull/26812#discussion_r1219568001)
Good point, dropped it!

💬 Sjors commented on pull request "guix: Clean up manifest":
(https://github.com/bitcoin/bitcoin/pull/27811#issuecomment-1578700432)
My guix build matches @TheCharlatan

💬 hebasto commented on pull request "guix: Clean up manifest":
(https://github.com/bitcoin/bitcoin/pull/27811#issuecomment-1578703813)
I'll be able to provide my own Guix build hashes tomorrow only.

💬 ismaelsadeeq commented on pull request "Fee estimation: avoid serving stale fee estimate":
(https://github.com/bitcoin/bitcoin/pull/27622#discussion_r1219595571)
I agree it's better than the magic number, I have updated it.

💬 ismaelsadeeq commented on pull request "Fee estimation: avoid serving stale fee estimate":
(https://github.com/bitcoin/bitcoin/pull/27622#discussion_r1219596235)
Fixed Thank you.

👍 instagibbs approved a pull request: "Fee estimation: avoid serving stale fee estimate"
(https://github.com/bitcoin/bitcoin/pull/27622#pullrequestreview-1465157911)
code review ACK 6d36f3edbe7549cfaadc48b43c09dd3e581e92ff

I'd really prefer we have a regtest-only option to skip the check to impact integration tests the least.

💬 instagibbs commented on issue "depriortisetransaction":
(https://github.com/bitcoin/bitcoin/issues/27807#issuecomment-1578737483)
> Yea exactly but that requires the user to track the txid and its delta or after https://github.com/bitcoin/bitcoin/pull/27501 is merged track the txid call getpriotisationmap and then use the negative delta from there
> was thinking it would be easier if the user could just specify the txid they want to remove

I tend to think that 27501 is enough? prioritization is a very advanced feature in itself, seems to be sufficient to me.

💬 instagibbs commented on pull request "test: Add -datacarriersize=2 tests":
(https://github.com/bitcoin/bitcoin/pull/27832#issuecomment-1578765163)
ACK https://github.com/bitcoin/bitcoin/pull/27832/commits/fa4f476d517669d42342223b5f03fb2025a12a74

👍 instagibbs approved a pull request: "mempool / rpc: add getprioritisedtransactions, delete a mapDeltas entry when delta==0"
(https://github.com/bitcoin/bitcoin/pull/27501#pullrequestreview-1465223614)
code review ACK 67b7fecacd0489809690982c89ba2d0acdca938c

just test suggestions, everything looks good

💬 instagibbs commented on pull request "mempool / rpc: add getprioritisedtransactions, delete a mapDeltas entry when delta==0":
(https://github.com/bitcoin/bitcoin/pull/27501#discussion_r1219665212)
is there a particular reason we're exposing in-mempool-ness here? No strong feelings, just curious.

💬 instagibbs commented on pull request "mempool / rpc: add getprioritisedtransactions, delete a mapDeltas entry when delta==0":
(https://github.com/bitcoin/bitcoin/pull/27501#discussion_r1219668280)
just directly construct the entire 2-entry map and assert here? means we wouldn't miss extraneous entries in future regression. Could also keep a running "tally" object that's modified along with the test cases

💬 instagibbs commented on pull request "mempool / rpc: add getprioritisedtransactions, delete a mapDeltas entry when delta==0":
(https://github.com/bitcoin/bitcoin/pull/27501#discussion_r1219669511)
same as above?

💬 instagibbs commented on pull request "mempool / rpc: add getprioritisedtransactions, delete a mapDeltas entry when delta==0":
(https://github.com/bitcoin/bitcoin/pull/27501#discussion_r1219669146)
same as above?

💬 instagibbs commented on pull request "mempool / rpc: add getprioritisedtransactions, delete a mapDeltas entry when delta==0":
(https://github.com/bitcoin/bitcoin/pull/27501#discussion_r1219680499)
maybe add a test case that shows prioritising by value of `0` doesn't add an entry, to cover all the bases

💬 kevkevinpal commented on issue "depriortisetransaction":
(https://github.com/bitcoin/bitcoin/issues/27807#issuecomment-1578836291)
> > Yea exactly but that requires the user to track the txid and its delta or after #27501 is merged track the txid call getpriotisationmap and then use the negative delta from there
> > was thinking it would be easier if the user could just specify the txid they want to remove
>
> I tend to think that 27501 is enough? prioritization is a very advanced feature in itself, seems to be sufficient to me.

ok yea makes sense and doesn't seem like anyone strongly wants this feature either, so I
...

✅ kevkevinpal closed an issue: "depriortisetransaction"
(https://github.com/bitcoin/bitcoin/issues/27807)

💬 petertodd commented on pull request "Remove -mempoolfullrbf option":
(https://github.com/bitcoin/bitcoin/pull/26525#issuecomment-1578839709)


On June 6, 2023 1:36:09 AM GMT+02:00, Antoine Riard ***@***.***> wrote:
>Note for the clarity, I think effectively both a loosening (e.g nVersion=3 where TX_MAX_STANDARD_VERSION allows nVersion=3 transaction propagation) and a tightening (e.g some nSequence field usage being banned from propagation) can be used to commit a double-spend.
>
>With a loosening, if the victim full-node is not upgraded, they won't accept your new nVersion=3 transactions, and you can feed the rest of the upgraded net
...

💬 petertodd commented on pull request "Remove -mempoolfullrbf option":
(https://github.com/bitcoin/bitcoin/pull/26525#issuecomment-1578839879)


On June 6, 2023 3:32:01 AM GMT+02:00, Anthony Towns ***@***.***> wrote:
>> Note for the clarity, I think effectively both a loosening (e.g nVersion=3 where TX_MAX_STANDARD_VERSION allows nVersion=3 transaction propagation) and a tightening (e.g some nSequence field usage being banned from propagation) can be used to commit a double-spend.
>
>Any mempool acceptance policy difference between two nodes can be used to prevent your node from seeing transactions the other node accepts. If you outrig
...