💬 fanquake commented on pull request "depends: reuse _config_opts for CMake options":
(https://github.com/bitcoin/bitcoin/pull/27496#issuecomment-1516365687)
Guix Build:
```bash
45d96de0248ec8b604cc5c980f1abbc780862d497f195a795c6088087d646c73 guix-build-63c0c4ff10b2/output/aarch64-linux-gnu/SHA256SUMS.part
ebdd087cbb45f1fa337b717c0d7bd53c8b7d0bc78d4e9693de2430a1ab9e03d9 guix-build-63c0c4ff10b2/output/aarch64-linux-gnu/bitcoin-63c0c4ff10b2-aarch64-linux-gnu-debug.tar.gz
e09206e6a70068036ea52731e0f8122665525a132e85f2798662ff9d7644dd8b guix-build-63c0c4ff10b2/output/aarch64-linux-gnu/bitcoin-63c0c4ff10b2-aarch64-linux-gnu.tar.gz
20e71234d2e90386
...

💬 MarcoFalke commented on pull request "test: Remove unused sanitizer suppressions":
(https://github.com/bitcoin/bitcoin/pull/27498#issuecomment-1516366010)
Force pushed to remove another unused one

💬 furszy commented on pull request "wallet: improve IBD sync time by skipping block scanning prior birth time":
(https://github.com/bitcoin/bitcoin/pull/27469#discussion_r1172623190)
> CBlock already has a GetBlockTime(), so I don't think it's necessary to include the time as a separate field here.

Yeah, that works here. The new field was because I initially implemented this on the filters-only sync branch. Where I have no blocks, only headers and filters.

> However I'm not sure that we should be using the block's timestamp for this time rather than MTP. With the block timestamp, we could ignore a more recent block that has a timestamp older than its parent which we wo
...

💬 furszy commented on pull request "wallet: improve IBD sync time by skipping block scanning prior birth time":
(https://github.com/bitcoin/bitcoin/pull/27469#discussion_r1172638173)
pushed thanks

💬 MarcoFalke commented on pull request "test: Remove unused sanitizer suppressions":
(https://github.com/bitcoin/bitcoin/pull/27498#issuecomment-1516416505)
I am thinking about optimistically adding a commit to remove the epoll_ctl one as well. If it still reproduces, it can be re-added?

💬 fanquake commented on pull request "test: Remove unused sanitizer suppressions":
(https://github.com/bitcoin/bitcoin/pull/27498#issuecomment-1516418849)
Might as well have a go

📝 glozow opened a pull request: "mempool / rpc: add getprioritisationmap, delete a mapDeltas entry when delta==0"
(https://github.com/bitcoin/bitcoin/pull/27501)
Add an RPC to get prioritised transactions (also tells you whether the tx is in mempool or not), helping users clean up `mapDeltas` manually. When `CTxMemPool::PrioritiseTransaction` sets a delta to 0, remove the entry from `mapDeltas`.

Motivation / Background
- `mapDeltas` entries are never removed from mapDeltas except when the tx is mined in a block or conflicted.
- Mostly it is a feature to allow `prioritisetransaction` for a tx that isn't in the mempool {yet, anymore}. A user can may
...

💬 glozow commented on pull request "mempool: keep CPFP'd transactions when loading from mempool.dat":
(https://github.com/bitcoin/bitcoin/pull/27476#issuecomment-1516503220)
**Alternatives Considered**

Feel free to suggest more alternatives, but note potential problems.

(1) Call TrimToSize() at smart intervals, e.g. when a timestamp changes (since packages are submitted with the same timestamp), or every ~25 transactions, or when we hit a certain threshold above maxmempool.
(2) When a transaction fails for fee-related reasons, try again with bypass_limits=true, and then schedule a flush for later.
(3) Create a new mempool.dat format, which contains a list of
...

💬 sipa commented on pull request "BIP324: ElligatorSwift integrations":
(https://github.com/bitcoin/bitcoin/pull/27479#discussion_r1172739143)
Done.

💬 MarcoFalke commented on pull request "mempool / rpc: add getprioritisationmap, delete a mapDeltas entry when delta==0":
(https://github.com/bitcoin/bitcoin/pull/27501#discussion_r1172733241)
```suggestion
"Returns a map of all fee deltas in memory pool by txid, and whether the tx is present in mempool.",
```

nit

👍 MarcoFalke approved a pull request: "mempool / rpc: add getprioritisationmap, delete a mapDeltas entry when delta==0"
(https://github.com/bitcoin/bitcoin/pull/27501#pullrequestreview-1394217946)
Concept ACK. Wanted to add a `deletepriority` RPC myself, but I guess it is easy to implement a for loop in python utilising the output of `getprioritisationmap`

💬 MarcoFalke commented on pull request "mempool / rpc: add getprioritisationmap, delete a mapDeltas entry when delta==0":
(https://github.com/bitcoin/bitcoin/pull/27501#discussion_r1172737216)
Won't this need mempool.cs to ensure an atomic result? Otherwise you may get nonsense back on a data race, like a parent missing from the mempool, when the child is in the mempool, no?

💬 sipa commented on pull request "BIP324: ElligatorSwift integrations":
(https://github.com/bitcoin/bitcoin/pull/27479#discussion_r1172739437)
Added some comments. Please have a look.

💬 sipa commented on pull request "BIP324: ElligatorSwift integrations":
(https://github.com/bitcoin/bitcoin/pull/27479#discussion_r1172739546)
Done.

💬 sipa commented on pull request "BIP324: ElligatorSwift integrations":
(https://github.com/bitcoin/bitcoin/pull/27479#discussion_r1172739678)
Done.

💬 sipa commented on pull request "BIP324: ElligatorSwift integrations":
(https://github.com/bitcoin/bitcoin/pull/27479#discussion_r1172739938)
Done. Also renamed to just `SIZE`.

💬 sipa commented on pull request "BIP324: ElligatorSwift integrations":
(https://github.com/bitcoin/bitcoin/pull/27479#discussion_r1172741182)
Your math is correct. And indeed, it's such a low probability event that it's not worth adding code to deal with it (even outside of a benchmark, I'd say). Still, I've added an assert to make it clear to the reader this is always supposed to be the case.

💬 glozow commented on pull request "mempool: keep CPFP'd transactions when loading from mempool.dat":
(https://github.com/bitcoin/bitcoin/pull/27476#issuecomment-1516508803)
> Maybe move most of the text in the PR description into a comment? "Review this first" and "Alternatives considered" stuff is useful here, but doesn't seem useful for the merge commit message?

Moved "alternatives considered." I prefer putting "review this first" at the top so reviewers see it as soon as possible. It will go away if/when I rebase, so definitely before any merging happens.

💬 instagibbs commented on pull request "BIP324: ElligatorSwift integrations":
(https://github.com/bitcoin/bitcoin/pull/27479#discussion_r1172742351)
significantly clearer, thanks

💬 glozow commented on pull request "mempool / rpc: add getprioritisationmap, delete a mapDeltas entry when delta==0":
(https://github.com/bitcoin/bitcoin/pull/27501#discussion_r1172747649)
Yes good point. Will add a lock-already-held `exists` and put this all under `LOCK(mempool.cs)`

💬 glozow commented on pull request "mempool / rpc: add getprioritisationmap, delete a mapDeltas entry when delta==0":
(https://github.com/bitcoin/bitcoin/pull/27501#discussion_r1172748986)
Oh, or would it be better to have `CTxMemPool::GetPrioritisationMap` take the lock and create a map from txid to pair<fee, bool>?