💬 theuni commented on pull request "crypto, refactor: add method for applying the taptweak":
(https://github.com/bitcoin/bitcoin/pull/30051#discussion_r1595390078)
Please consider the above a doodle, change it however you'd like. I just wanted to sketch out the concept of a wrapped `keypair`.

💬 TheCharlatan commented on pull request "kernel: Remove key module from kernel library":
(https://github.com/bitcoin/bitcoin/pull/29252#issuecomment-2102601696)
Thank you for the review @ryanofsky!

a885a166cec6d84d08600f12b25d912bd28af80e -> 7dc7938731233f5f3299618e1e07ff92782a7cfd ([kernelRmKey_2](https://github.com/TheCharlatan/bitcoin/tree/kernelRmKey_2) -> [kernelRmKey_3](https://github.com/TheCharlatan/bitcoin/tree/kernelRmKey_3), [compare](https://github.com/TheCharlatan/bitcoin/compare/kernelRmKey_2..kernelRmKey_3))
* Addressed @ryanofsky's [comment](https://github.com/bitcoin/bitcoin/pull/29252#pullrequestreview-2042988944), cherry-picked th
...

💬 josibake commented on pull request "crypto, refactor: add method for applying the taptweak":
(https://github.com/bitcoin/bitcoin/pull/30051#discussion_r1595408180)
Yeah, for sure! But thanks, very helpful doodle. Not sure why I was so apprehensive about passing around a keypair, but seeing you write it out helped it click for me.

💬 TheCharlatan commented on pull request "kernel: Remove key module from kernel library":
(https://github.com/bitcoin/bitcoin/pull/29252#issuecomment-2102676859)
Rebased 7dc7938731233f5f3299618e1e07ff92782a7cfd -> 65a205166b3c7d082a7528ea5036dff551d7eb6e ([kernelRmKey_3](https://github.com/TheCharlatan/bitcoin/tree/kernelRmKey_3) -> [kernelRmKey_4](https://github.com/TheCharlatan/bitcoin/tree/kernelRmKey_4), [compare](https://github.com/TheCharlatan/bitcoin/compare/kernelRmKey_3..kernelRmKey_4))

💬 vasild commented on pull request "rpc: provide per message stats for global traffic via new RPC 'getnetmsgstats'":
(https://github.com/bitcoin/bitcoin/pull/29418#issuecomment-2102685727)
`91c90d7595...6bfda84b92`: rebase, pick changes from https://github.com/bitcoin/bitcoin/pull/29421 and adjust the newly added test.

👍 ryanofsky approved a pull request: "kernel: Remove key module from kernel library"
(https://github.com/bitcoin/bitcoin/pull/29252#pullrequestreview-2047986415)
Code review ACK 65a205166b3c7d082a7528ea5036dff551d7eb6e

💬 ryanofsky commented on pull request "kernel: Remove key module from kernel library":
(https://github.com/bitcoin/bitcoin/pull/29252#discussion_r1595470766)
In commit "Refactor: Remove ECC_Start and ECC_Stop from key header" (65a205166b3c7d082a7528ea5036dff551d7eb6e)

It's probably good to remove these declarations, but I think it would be good to move the documentation to the cpp file so it is not lost. Notes like "May not be called twice" "No-op if ECC_Start wasn't called" seem useful to describe how these are intended to work.

Also references to these function in the ECC_Context comment below should be removed if they are private. Maybe repl
...

💬 instagibbs commented on pull request "test: add conflicting topology test case":
(https://github.com/bitcoin/bitcoin/pull/30066#discussion_r1595483759)
and specifically, the conflict is happening between a transaction in the mempool already, and a transaction in the package

💬 Sjors commented on pull request "net: Replace libnatpmp with built-in PCP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#issuecomment-2102707649)
In terms of which IPv6 address to use, my understanding is this:

1. In the olden days the IPv6 address contained part of the MAC address. This ensured uniqueness, but was bad for mobile device privacy; no matter where you went, part of your IP address was constant. Such addresses always (?) contain `0xfffe`. See https://datatracker.ietf.org/doc/html/rfc7707
2. A new standard was introduced that uses a hash of both the MAC address and the prefix, ensuring it's stable if you stay on the same n
...

🤔 brunoerg reviewed a pull request: "test: add MiniWallet tagging support to avoid UTXO mixing, use in `fill_mempool`"
(https://github.com/bitcoin/bitcoin/pull/29939#pullrequestreview-2048018941)
utACK dd8fa861939d5b8bdd844ad7cab015d08533a91a

Code lgtm and I do not see any problem with the "tag", and seems good for determinism.

📝 instagibbs opened a pull request: "refactor prep for package rbf"
(https://github.com/bitcoin/bitcoin/pull/30072)
First few commits of https://github.com/bitcoin/bitcoin/pull/28984 to set the stage for the package RBF logic.

No behavior changes aside from bailing earlier from failed carve-outs.

💬 instagibbs commented on pull request "refactor prep for package rbf":
(https://github.com/bitcoin/bitcoin/pull/30072#issuecomment-2102717549)
suggested by @glozow , can close if this ends up not being helpful

💬 glozow commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1595480359)
b86487ccf2e3d43da3d3ce8c3d2e2510677eb9ec

I wonder if `PaysForRBF` and `ImprovesFeerateDiagram` should be swapped in the order of checks?

`ImprovesFeerateDiagram` is a more expensive, is a superset of the rules, and a less intuitive error (I'd have an easier time guessing what went wrong with "pays less fees" vs "does not improve feerate diagram". Especially looking at the functional tests which switched out the error strings due to this check being earlier.

💬 glozow commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1595484318)
```suggestion
return package_state.Invalid(PackageValidationResult::PCKG_POLICY, "package RBF failed: new transaction cannot have mempool ancestors");
```

💬 glozow commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1595472969)
in b86487ccf2e3d43da3d3ce8c3d2e2510677eb9ec

The comments + error string are a bit contradictory ("child only paying anti-DoS" and "parent paying for child anti-DoS") unless maybe I'm misunderstanding?

Also maybe error str "package RBF failed: parent paying for child replacement" and debug str "package feerate is X while parent feerate is Y" ?

💬 glozow commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1595487038)
Maybe you can mention that this replaces `PaysMoreForConflicts` function from `ReplacementChecks`?

💬 glozow commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1595494866)
Could add more detailed docs for what the rules are in package RBF?

- all to-be-replaced signal (bip125 or v3)
- package is 1p1c, only in clusters up to size 2
- to-be-replaced all in clusters up to size 2
- no more than 100 total to-be-replaced
- total fees of package > all fees being replaced, at incremental relay feerate
- must improve feerate diagram
- parent feerate must be < package feerate

💬 glozow commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1595176618)
yeah, I think `PCKG_POLICY` is fine given https://github.com/bitcoin/bitcoin/pull/30012/commits/6119f76ef72c1adc55c1a384c20f8ba9e1a01206. Just need to make sure the `MempoolAcceptResult`s are correct so that we are attributing the failures correctly

💬 glozow commented on pull request "Cluster size 2 package rbf":
(https://github.com/bitcoin/bitcoin/pull/28984#discussion_r1595490481)
I don't think it's very clear which cluster isn't size two in this str
```suggestion
return package_state.Invalid(PackageValidationResult::PCKG_POLICY, "package RBF failed: package must be 1-parent-1-child");
```

💬 TheCharlatan commented on pull request "kernel: Remove key module from kernel library":
(https://github.com/bitcoin/bitcoin/pull/29252#issuecomment-2102723104)
Updated 65a205166b3c7d082a7528ea5036dff551d7eb6e -> 96378fe734e5fb6167eb20036d7170572a647edb ([kernelRmKey_4](https://github.com/TheCharlatan/bitcoin/tree/kernelRmKey_4) -> [kernelRmKey_5](https://github.com/TheCharlatan/bitcoin/tree/kernelRmKey_5), [compare](https://github.com/TheCharlatan/bitcoin/compare/kernelRmKey_4..kernelRmKey_5))

* Addressed @ryanofsky's [comment](https://github.com/bitcoin/bitcoin/pull/29252#discussion_r1595470766), moving comments and improving description of `ECC_Co
...