💬 gmaxwell commented on pull request "Reduce minrelaytxfee to 100 sats/kvB":
(https://github.com/bitcoin/bitcoin/pull/32959#issuecomment-3079827081)
> I assume the same logic holds for incremental rate?

I think that the incremental rate, like dust limit, would in an ideal world be just a pure function of the "actual" feerate, but we don't have access to the actual feerate and the actual rate is fluctuating while it's preferable for these to be more stable so the minrelay fee gets used as a proxy.

> Not sure I understood this point.

If there is a sustained period of low demand (such that few blocks are full) then absent artificial li
...

🤔 w0xlt reviewed a pull request: "wallet: Set migrated wallet name only on success"
(https://github.com/bitcoin/bitcoin/pull/32984#pullrequestreview-3026426274)
ACK https://github.com/bitcoin/bitcoin/pull/32984/commits/3a0be655c4fd5c74731f1fcb57c45e6a8c3c362f

💬 instagibbs commented on pull request "validation: docs and cleanups for MemPoolAccept coins views":
(https://github.com/bitcoin/bitcoin/pull/32973#discussion_r2211313016)
s/has now been spent/has now been spent in the mempool/ ?

💬 glozow commented on pull request "wallet, rpc: add v3 transaction creation and wallet support":
(https://github.com/bitcoin/bitcoin/pull/32896#discussion_r2211351922)
maybe these should live in mempool_util?

💬 glozow commented on pull request "wallet, rpc: add v3 transaction creation and wallet support":
(https://github.com/bitcoin/bitcoin/pull/32896#discussion_r2211362713)
Might want to assert that it's v2

💬 glozow commented on pull request "wallet, rpc: add v3 transaction creation and wallet support":
(https://github.com/bitcoin/bitcoin/pull/32896#discussion_r2211355006)
This looks like it was copy-pasted from the test framework - can we just use the existing method, perhaps with a wrapper if needed?

💬 glozow commented on pull request "wallet, rpc: add v3 transaction creation and wallet support":
(https://github.com/bitcoin/bitcoin/pull/32896#discussion_r2211375162)
Could you also add `v2_tx_spends_confirmed_v3_tx` and `v3_tx_spends_confirmed_v2_tx` to check that version mismatches are fine there?

💬 glozow commented on pull request "wallet, rpc: add v3 transaction creation and wallet support":
(https://github.com/bitcoin/bitcoin/pull/32896#discussion_r2211372152)
Should there also be a test where Alice tries to spend her change after Bob has spent from the parent? That wouldn't require `include_unsafe` IIUC

💬 glozow commented on pull request "wallet, rpc: add v3 transaction creation and wallet support":
(https://github.com/bitcoin/bitcoin/pull/32896#discussion_r2211379860)
What does "two outputs" mean here? This has 1 output, no?

💬 glozow commented on pull request "wallet, rpc: add v3 transaction creation and wallet support":
(https://github.com/bitcoin/bitcoin/pull/32896#discussion_r2211390451)
Isn't this just...?
```suggestion
alice_v2_unspent = self.alice.listunspent(minconf=1)[0]
```

💬 glozow commented on pull request "wallet, rpc: add v3 transaction creation and wallet support":
(https://github.com/bitcoin/bitcoin/pull/32896#discussion_r2211381702)
Will Alice's wallet attempt to rebroadcast her transaction if/when the parent + Bob's confirm? Can we do an RBF of Alice's transaction to evict Bob's?

💬 glozow commented on pull request "wallet, rpc: add v3 transaction creation and wallet support":
(https://github.com/bitcoin/bitcoin/pull/32896#discussion_r2211385875)
```suggestion
# send a v2 output to alice and confirm it
````

💬 glozow commented on pull request "wallet, rpc: add v3 transaction creation and wallet support":
(https://github.com/bitcoin/bitcoin/pull/32896#discussion_r2211403206)
Can this be consolidated with `v3_conflict_removed_from_mempool` since they are mostly the same?

🤔 w0xlt reviewed a pull request: "rpc: refactor: use string_view in Arg/MaybeArg"
(https://github.com/bitcoin/bitcoin/pull/32983#pullrequestreview-3026746517)
ACK https://github.com/bitcoin/bitcoin/pull/32983/commits/319abd609b3d2194e7a05454b6af55b6f16adbf1

🤔 l0rinc reviewed a pull request: "[IBD] multi-byte block obfuscation"
(https://github.com/bitcoin/bitcoin/pull/31144#pullrequestreview-3026774129)
Thanks for the hints, the latest version simplifies reading the obfuscation key by using the new `Obfuscation` object's serialization paths + reading back the new key directly after generating a new one.

The `obfuscation_constructors` test was also changes to validate the new `HexKey` conversion which we need since we're logging the generated obfuscations keys so we need to peek into the object.

It also avoids a possible `Obfuscation` copy used in `GetObfuscation` and removed the `obfuscat
...

💬 l0rinc commented on pull request "[IBD] multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2211528848)
Done

💬 l0rinc commented on pull request "[IBD] multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2211529426)
Let me know what you think of the result now.

💬 l0rinc commented on pull request "[IBD] multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2211529189)
> m_obfuscation = Obfuscation{FastRandomContext{}.randbytes<8>()};
Write(OBFUSCATION_KEY_KEY, m_obfuscation);

If we enable obfuscation (by assigning the field) before calling `Write`, it will obfuscate the new key *with* the new key in `CDBBatch::WriteImpl`, so before writing we need `m_obfuscation` to remain empty. I've extended the comments to clarify that.

> FastRandomContext calls GetRandBytes internally

Nice, if we can indeed use `FastRandomContext{}`, the code can be simplified a
...

💬 l0rinc commented on pull request "[IBD] multi-byte block obfuscation":
(https://github.com/bitcoin/bitcoin/pull/31144#discussion_r2211529040)
Removed from intermediary commit

🤔 ryanofsky reviewed a pull request: "[IBD] multi-byte block obfuscation"
(https://github.com/bitcoin/bitcoin/pull/31144#pullrequestreview-3026270097)
Code review 7b93fa394acaee7a715fae9a2b968d33c5adc174. Still in progress reviewing but wanted to post comments I had so far. There was a new push since these were written so please ignore anything that no longer applies.