💬 TheCharlatan commented on pull request "multiprocess: Add capnp wrapper for Chain interface":
(https://github.com/bitcoin/bitcoin/pull/29409#discussion_r1834309312)
Looking at the usage of this endpoint I am wondering if it is even necessary? I.e. instead of getting it first and using it for `chainStateFlushed`, the `chainStateFlushed` implementation could just do that internally.

More generally I have been wondering if it would be a good idea to eventually strongly type these `Data` blobs (and similarly the enums)? For example here, wrapping it in a `BlockLocator` type. I don't think this is relevant as long as libmultiprocess is used to create the c++
...

💬 TheCharlatan commented on pull request "multiprocess: Add capnp wrapper for Chain interface":
(https://github.com/bitcoin/bitcoin/pull/29409#discussion_r1834348410)
Nit: Shouldn't this be called `txid`?

💬 TheCharlatan commented on pull request "multiprocess: Add capnp wrapper for Chain interface":
(https://github.com/bitcoin/bitcoin/pull/29409#discussion_r1834365825)
Why is the `role` typed as `UInt32` when the other enums are typed as `Int32`?

💬 TheCharlatan commented on pull request "multiprocess: Add capnp wrapper for Chain interface":
(https://github.com/bitcoin/bitcoin/pull/29409#discussion_r1834376651)
Should this include the version as well at this point?

💬 TheCharlatan commented on pull request "multiprocess: Add capnp wrapper for Chain interface":
(https://github.com/bitcoin/bitcoin/pull/29409#discussion_r1834362068)
How does this `write` argument here map to the `action` argument in the c++ interface? They seem to have different types.

👍 BrandonOdiwuor approved a pull request: "test: Add combinerawtransaction test to rpc_createmultisig"
(https://github.com/bitcoin/bitcoin/pull/31249#pullrequestreview-2423781623)
Code Review ACK af4d23178b420f46196fbace2176ce1fe94ed9cd

💬 dergoegge commented on pull request "ci: Split out native fuzz jobs for macOS and windows (take 2)":
(https://github.com/bitcoin/bitcoin/pull/31221#issuecomment-2464747471)
@hebasto could you give this another look?

💬 dergoegge commented on pull request "doc: recitfy typos":
(https://github.com/bitcoin/bitcoin/pull/31253#issuecomment-2464751924)
Same as #30259?

💬 maflcko commented on pull request "rpc, logging: return "verificationprogress" of 1 when up to date":
(https://github.com/bitcoin/bitcoin/pull/31177#discussion_r1834398930)
> Hi, I'm not sure if I understood you correctly, but I guess you say it because it's possible to call the function with `hindex = nullptr`.

I am asking why it needs to be passed at all. Shouldn't it be the same for all callers?

💬 fanquake commented on pull request "build: increase minimum supported Windows to 10.0":
(https://github.com/bitcoin/bitcoin/pull/31172#discussion_r1834418655)
I've just shortened the comment here.

💬 maflcko commented on pull request "tinyformat: enforce compile-time checks for format string literals":
(https://github.com/bitcoin/bitcoin/pull/31149#issuecomment-2464789937)
I agree that the changes here are still useful. It would be good to rebase on top of https://github.com/bitcoin/bitcoin/pull/31174, maybe after it is merged.

💬 polespinasa commented on pull request "rpc, logging: return "verificationprogress" of 1 when up to date":
(https://github.com/bitcoin/bitcoin/pull/31177#discussion_r1834442999)
Oh, understood, yes, I think so.
But it's the way to not move the function inside ``chainman`` and keep the actual structure. Do you think is more reasonable move it inside?

👍 hodlinator approved a pull request: "tinyformat: Add compile-time checking for literal format strings"
(https://github.com/bitcoin/bitcoin/pull/31174#pullrequestreview-2423920012)
ACK ecc5cb9a89c6b001df839675b23d8fc1f7ac69ba

Implemented my suggestions (except comment removal suggestion) + broke out `parse_size()` since my last review.

*util_string_tests* tests passed locally.

Left one comment, but nothing blocking.

💬 hodlinator commented on pull request "tinyformat: Add compile-time checking for literal format strings":
(https://github.com/bitcoin/bitcoin/pull/31174#discussion_r1834470480)
sdfsdfsd

💬 hodlinator commented on pull request "tinyformat: Add compile-time checking for literal format strings":
(https://github.com/bitcoin/bitcoin/pull/31174#discussion_r1834472950)
As you say, only the length and type are no longer checked. This PR now implements rudimentary checking of flags, width and precision.
```suggestion
// Length and type in "[flags][width][.precision][length]type"
// is not checked. Parsing continues with the next '%'.
```

💬 darosior commented on pull request "descriptor: Add proper Clone function to miniscript::Node":
(https://github.com/bitcoin/bitcoin/pull/30866#discussion_r1834499475)
This is not necessary anymore.

📝 naiyoma opened a pull request: "feature: RPC show decompiled P2SH redeemScript and P2WSH witnessScript"
(https://github.com/bitcoin/bitcoin/pull/31256)
Closes #27391

Add support for displaying the (decompiled) P2SH redeemScript and P2WSH
witnessScript when calling `getrawtransaction and getblock....2.`


I used this [guide](https://naiyoma.github.io/) to create transactions and below are outputs from p2sh/p2wsh , getrawtransaction and getblock 2 on regtest
```
bitcoin-cli -regtest getrawtransaction $TXID 1 | jq -r '.vin[].redeemScript.asm

2 0332df518c53eba135966d45b350e7a9bbf984
...

💬 fanquake commented on pull request "feature: RPC show decompiled P2SH redeemScript and P2WSH witnessScript":
(https://github.com/bitcoin/bitcoin/pull/31256#issuecomment-2464920585)
I think this is already being worked on in #31252.

✅ naiyoma closed a pull request: "feature: RPC show decompiled P2SH redeemScript and P2WSH witnessScript"
(https://github.com/bitcoin/bitcoin/pull/31256)

💬 polespinasa commented on pull request "feature: RPC show decompiled P2SH redeemScript and P2WSH witnessScript":
(https://github.com/bitcoin/bitcoin/pull/31256#issuecomment-2464938959)
> I think this is already being worked on in #31252.

@naiyoma please feel free to contribute in my PR if you want to work on this. P2SH still on TODO, only P2WSH is implemented and solving functional test still also on TODO.