💬 achow101 commented on issue "RFC: Formal description of the RPC API":
(https://github.com/bitcoin/bitcoin/issues/29912#issuecomment-3282946065)
@laanwj @casey Are either of you still working on a PR for this?

💬 davidgumberg commented on pull request "cmake: fatal error when PIE not supported":
(https://github.com/bitcoin/bitcoin/pull/33282#issuecomment-3283024096)
> > I think the current behaviour of using PIE for all targets is sensible
>
> I am not convinced and I really would like to know the rationale for that. Maybe @hebasto or @theuni knows?

For motivation, you can look to the commit which enabled PIE by default: https://github.com/bitcoin/bitcoin/commit/3f94dfa25fc1b0e838d368a9b2683a634c.

In order for operating systems employing ASLR to randomize the base address of an executable's `text` segment, binaries must be built as PIE, and this be
...

🤔 w0xlt reviewed a pull request: "log: always print initial signature verification state"
(https://github.com/bitcoin/bitcoin/pull/33336#pullrequestreview-3214063456)
ACK https://github.com/bitcoin/bitcoin/pull/33336/commits/39f41135c764c6c4705ce2cd36781ca8d43f0114

Simple test on mainnet:
```
./build/bin/bitcoind -datadir=new_data_dir/ -assumevalid=000000009b7262315dbf071787ad3656097b892abffd1f95a1a022f896f533fc -stopatheight=10
```

👍 davidgumberg approved a pull request: "test: fix p2p_leak_tx.py"
(https://github.com/bitcoin/bitcoin/pull/33121#pullrequestreview-3214061871)
ACK e14825ace2a8c6abcf11fdfb

- (https://github.com/bitcoin/bitcoin/pull/33121/commits/d73952df9e755f1349ff4a19c47ffc92efa3a2b1) Mitigates the likeliest cause of the spurious failure #33090 in `p2p_leak_tx.py` by increasing the timeout we'll wait for an INV.
- (https://github.com/bitcoin/bitcoin/pull/33121/commits/3005fb2ad39aec6fdb52dc6f406672a9515934ca) `test_notfound_on_unannounced_tx` previously was not really testing anything, as written in the PR description:
> we send a MSG_TX-t
...

💬 davidgumberg commented on pull request "test: fix p2p_leak_tx.py":
(https://github.com/bitcoin/bitcoin/pull/33121#discussion_r2342619001)
nanonit, only if retouching:

```suggestion
inbound_peer.wait_for_inv([CInv(t=MSG_WTX, h=wtxid)], timeout=120)
```

💬 ajtowns commented on pull request "Bump SCRIPT_VERIFY flags to 64 bit":
(https://github.com/bitcoin/bitcoin/pull/32998#discussion_r2342637583)
The rule of 5 just says if you define one, define/delete them all; I do it because that ensures I get a compile time error if there's some problem that makes one of them not possible. Move semantics probably aren't very interesting for a wrapper around an int though.

The C++ Core Guidelines version of the ["rule of 0"](https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Rc-zero) says not to define any of them when possible, however.

💬 ajtowns commented on pull request "Bump SCRIPT_VERIFY flags to 64 bit":
(https://github.com/bitcoin/bitcoin/pull/32998#discussion_r2342638488)
Note that it's consteval, so the throw is a compile-time check, not a runtime one.

💬 davidgumberg commented on pull request "build: Remove deprecated CMAKE_SKIP_BUILD_RPATH and SKIP_BUILD_RPATH settings":
(https://github.com/bitcoin/bitcoin/pull/33247#issuecomment-3283079195)
This CI failure is spurious, see https://github.com/bitcoin/bitcoin/issues/33345.

To force CI to rerun you can change your commit's timestamp (and hash) by doing `git commit --amend --no-edit` and push it.

💬 ajtowns commented on pull request "validation: ensure assumevalid is always used during reindex":
(https://github.com/bitcoin/bitcoin/pull/31615#discussion_r2342658393)
The minimumchainwork [should match](https://github.com/bitcoin-core/bitcoin-maintainer-tools/blob/f36fa930bb99e332f2ccb9f76a42ea721850feb8/release-prep.sh#L151-L160) the assumevalid block, so if minimumchainwork isn't reached the assumevalid block won't be present and this code block won't be executed.

So this is only relevant for reindexing when you're manually choosing an older assumevalid block than the default, and aren't also adjusting the minchainwork. (If you choose a more recent assum
...

💬 yorick1125 commented on pull request "ci: always use tag for LLVM checkout":
(https://github.com/bitcoin/bitcoin/pull/33364#issuecomment-3283383056)
,

💬 casey commented on issue "RFC: Formal description of the RPC API":
(https://github.com/bitcoin/bitcoin/issues/29912#issuecomment-3283403636)
@achow101 I sort of lost steam, although I did take a couple of stabs at it.

[This branch](https://github.com/casey/bitcoin/tree/schema-rpc) adds a `schema` RPC which outputs a JSON description of all RPCs. You can see the output [here](https://github.com/casey/bitcoin/blob/9d90e50d191330237688eea1a4bd93018b417da2/schema.json). The format is ad-hoc.

Based on feedback that it might be preferable to use a standard format, I started on an implementation which uses JSON Schema in [this branch](ht
...

💬 pablomartin4btc commented on pull request "rpc: refactor: use string_view in Arg/MaybeArg":
(https://github.com/bitcoin/bitcoin/pull/32983#discussion_r2342778880)
not sure about the clarity of this one-liner change... perhaps could be:
```suggestion
if (auto dummy = self.MaybeArg<std::string_view>("dummy"); dummy && *dummy != "*") {
```

💬 pablomartin4btc commented on pull request "rpc: refactor: use string_view in Arg/MaybeArg":
(https://github.com/bitcoin/bitcoin/pull/32983#discussion_r2342779089)
nit: (not a blocker) if you need to re-touch...
```suggestion
if (!descriptor.empty() && !ret.empty()) {
```

💬 pablomartin4btc commented on pull request "rpc: refactor: use string_view in Arg/MaybeArg":
(https://github.com/bitcoin/bitcoin/pull/32983#discussion_r2342779257)
nit (for safety & consistency):
```suggestion
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, tfm::format"Unknown address type '%s'", address_type));
```

Already done in other places like:

https://github.com/bitcoin/bitcoin/blob/b49a4f17aba76d2f2d7f1109d7e68b02303947bf/src/rpc/node.cpp#L193

💬 pablomartin4btc commented on pull request "rpc: refactor: use string_view in Arg/MaybeArg":
(https://github.com/bitcoin/bitcoin/pull/32983#discussion_r2342779437)
nit (same observation here regarding `tfm::format`):
```suggestion
throw JSONRPCError(RPC_INVALID_PARAMETER, tfm::format("Invalid action '%s'", action));
```

👍 pablomartin4btc approved a pull request: "rpc: refactor: use string_view in Arg/MaybeArg"
(https://github.com/bitcoin/bitcoin/pull/32983#pullrequestreview-3214377448)
crACK & tACK b49a4f17aba76d2f2d7f1109d7e68b02303947bf

Verified addressed comments & suggestions from @maflcko since my last [review](https://github.com/bitcoin/bitcoin/pull/32983#pullrequestreview-3051797902).

Performed some manual testing on RPC calls (`getblock`, `addnode`, `getdescriptoractivity`, `signmessage`, `verifymessage`). Also tested them passing very long strings (>10MB).

Left a couple of tiny nits, none blocker.

💬 l0rinc commented on pull request "common: Make arith_uint256 trivially copyable":
(https://github.com/bitcoin/bitcoin/pull/33332#issuecomment-3283440999)
> let's run some benchmarks

I have already tested move construction in https://github.com/bitcoin/bitcoin/pull/33332#pullrequestreview-3197960566

💬 nguyenthingot1973v-spec commented on issue "把虚拟的“挖矿”货币应用到真实的实体“挖有机矿”，采用IoT设备记录真实的生产数据构成NFT，实现可追溯，不可簒改，去中心化的区块链Pi公链分布式":
(https://github.com/bitcoin/bitcoin/issues/33357#issuecomment-3283442470)
****

💬 l0rinc commented on pull request "RFC: blocks: add `-reobfuscate-blocks` arg to xor existing blk/rev on startup":
(https://github.com/bitcoin/bitcoin/pull/33324#discussion_r2342879651)
I had that version before, but didn't like that the small and big files made the percentages look unevenly spaced. But I have reverted that version and shuffled the files, this should make the progress feel more uniform - thank you for the observation!

💬 l0rinc commented on pull request "RFC: blocks: add `-reobfuscate-blocks` arg to xor existing blk/rev on startup":
(https://github.com/bitcoin/bitcoin/pull/33324#discussion_r2342880343)
Done, thanks!