💬 pinheadmz commented on pull request "rpc: show P2(W)SH redeemScript in getrawtransaction #27637":
(https://github.com/bitcoin/bitcoin/pull/27638#issuecomment-1648360532)
There are some hints in `test/rpc_createmultisig.py` and probably a few other tests as well. I also tried to create a P2SH multisig and spend from it manually on regtest, it is quite involved...

https://gist.github.com/pinheadmz/2e85bbdd1b21ed2946f84a88d38f172a

👍 brunoerg approved a pull request: "test: cover addrv2 anchors by adding TorV3 to CAddress in messages.py"
(https://github.com/bitcoin/bitcoin/pull/27452#pullrequestreview-1544032294)
crACK ba8ab4fc545800c4fb283a5ff0b19138a0451aba

code lgtm!

🤔 jaonoctus requested changes to a pull request: "policy: Enable full-rbf by default"
(https://github.com/bitcoin/bitcoin/pull/28132#pullrequestreview-1544055502)
nACK

> That's 37% of total hash power regularly mining full-rbf transactions.

I think this decision is premature and that is not enough for the current value to be changed, honestly. Still prefer it as opt-in instead of enabled by default ATM

> Let's get this merged and this silly debate over with. Miners and node operators who choose to disable full-rbf still can with a simple configuration change.

Let's get this not merged and this silly debate over with. Miners and node operator
...

💬 TheCharlatan commented on pull request "kernel: Remove UniValue from kernel library":
(https://github.com/bitcoin/bitcoin/pull/28113#issuecomment-1648426036)
Updated a3774d1b2a5ce9aa6d6d3cedc2c9b9a5d2f68240 -> 42233bea28f6f7c464f0cd6499d675e81b3e8512 ([kernelRmUnivalue_5](https://github.com/TheCharlatan/bitcoin/tree/kernelRmUnivalue_5) -> [kernelRmUnivalue_6](https://github.com/TheCharlatan/bitcoin/tree/kernelRmUnivalue_6), [compare](https://github.com/TheCharlatan/bitcoin/compare/kernelRmUnivalue_5..kernelRmUnivalue_6))

* Addressed @stickies-v's [comment](https://github.com/bitcoin/bitcoin/pull/28113#discussion_r1272218244), pushing the handling
...

💬 TheCharlatan commented on pull request "kernel: Remove UniValue from kernel library":
(https://github.com/bitcoin/bitcoin/pull/28113#discussion_r1272628943)
@ theuni, I did not go with your suggestion, but instead opted to move the weird default handling to the calling function. I feel like that should be the caller's responsibility. Also did not go with the out result type, since the function signatures are not really consistent to begin with, and we don't have an out result for the corresponding `SighashToStr` function either.

👍 TheCharlatan approved a pull request: "net processing, refactor: Decouple PeerManager from gArgs"
(https://github.com/bitcoin/bitcoin/pull/27499#pullrequestreview-1544070845)
ACK 23c7b51ddd2888cf7fb260c439f004bd28768473

💬 ishaanam commented on pull request "test: create wallet specific for test_locked_wallet case":
(https://github.com/bitcoin/bitcoin/pull/28139#issuecomment-1648456270)
utACK c648bdbda21c7ae90c6b40e506ca4ed62b1dbb6c

💬 TheCharlatan commented on pull request "rpc, util: deduplicate AmountFromValue() using util::Result":
(https://github.com/bitcoin/bitcoin/pull/28134#discussion_r1272645850)
Including `rpc/` anything seems unfortunate to do here. Maybe these UniValue-specific utilities should instead be moved to their own files?

🤔 mzumsande reviewed a pull request: "Rework validation logic for assumeutxo"
(https://github.com/bitcoin/bitcoin/pull/27746#pullrequestreview-1544102750)
Code Review ACK 137762f34a845e491b80f9cea07efc4427cb38bf

I reviewed all commits again and did a bit of testing with snapshots on signet.

💬 darosior commented on pull request "fuzz: Test headers pre-sync through p2p interface":
(https://github.com/bitcoin/bitcoin/pull/28043#discussion_r1272659160)
Why not disabling it completely instead of mocking it?

👍 theuni approved a pull request: "net processing, refactor: Decouple PeerManager from gArgs"
(https://github.com/bitcoin/bitcoin/pull/27499#pullrequestreview-1544114932)
Concept ACK and quick review ACK.

My only nit is that `size_t max_extra_txs` is platform dependent and says nothing about its upper bound. Similarly, from a quick glance, I think `max_orphan_txs` could wrap from the command line?

(I realize these aren't new problems, they're just highlighted here)

Maybe as a followup we could add some pedantic checks in init to make sure they're sane values that fit into 32bits and make `max_orphan_txs` a `uint32_t`. Please ignore if those checks exist
...

💬 theuni commented on pull request "kernel: Remove UniValue from kernel library":
(https://github.com/bitcoin/bitcoin/pull/28113#discussion_r1272674822)
I like this, thanks! It just looks like a more modern version of the other functions. I also like that the weird no-value case is handled at the JSON layer instead. That's indeed cleaner than what I suggested.

📝 donCallm opened a pull request: "[TASK 0] FIRST CHANGE IN MY BLOCKCHAIN"
(https://github.com/bitcoin/bitcoin/pull/28140)
<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements or new tests that improv
...

✅ fanquake closed a pull request: "[TASK 0] FIRST CHANGE IN MY BLOCKCHAIN"
(https://github.com/bitcoin/bitcoin/pull/28140)

📝 fanquake locked a pull request: "[TASK 0] FIRST CHANGE IN MY BLOCKCHAIN"
(https://github.com/bitcoin/bitcoin/pull/28140)
<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements or new tests that improv
...

📝 donCallm opened a pull request: "[TASK 0] FIRST CHANGE IN MY BLOCKCHAIN"
(https://github.com/bitcoin/bitcoin/pull/28141)
<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements or new tests that improv
...

✅ pinheadmz closed a pull request: "[TASK 0] FIRST CHANGE IN MY BLOCKCHAIN"
(https://github.com/bitcoin/bitcoin/pull/28141)

📝 fanquake locked a pull request: "[TASK 0] FIRST CHANGE IN MY BLOCKCHAIN"
(https://github.com/bitcoin/bitcoin/pull/28141)
<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

* Any test improvements or new tests that improv
...

👍 theuni approved a pull request: "kernel: Remove UniValue from kernel library"
(https://github.com/bitcoin/bitcoin/pull/28113#pullrequestreview-1544154683)
ACK 42233bea28f6f7c464f0cd6499d675e81b3e8512

Assuming CI is happy.

Very nice removal :)

💬 theuni commented on pull request "kernel: Remove UniValue from kernel library":
(https://github.com/bitcoin/bitcoin/pull/28113#discussion_r1272685574)
Woohoo!