💬 achow101 commented on pull request "wallet, rpc: add anti-fee-sniping to `send` and `sendall`":
(https://github.com/bitcoin/bitcoin/pull/28944#discussion_r1414538735)
In 5101f054ce435ac653a3f5e780fca066e2f25088 "test: test sendall and send do anti-fee-sniping"

nit: `gettransaction` has a `verbose` argument that does the decoding so this doesn't need to call `decoderawtransaction`. Same below.

💬 achow101 commented on pull request "wallet, rpc: add anti-fee-sniping to `send` and `sendall`":
(https://github.com/bitcoin/bitcoin/pull/28944#discussion_r1414539439)
In 5101f054ce435ac653a3f5e780fca066e2f25088 "test: test sendall and send do anti-fee-sniping"

nit: `getrawtransaction` has a `verbosity` option that will do the decoding, so no need to call `decoderawtransaction` here.

💬 achow101 commented on pull request "wallet, rpc: add anti-fee-sniping to `send` and `sendall`":
(https://github.com/bitcoin/bitcoin/pull/28944#discussion_r1414539685)
In 5101f054ce435ac653a3f5e780fca066e2f25088 "test: test sendall and send do anti-fee-sniping"

Shouldn't this check for `getblockcount() - 100`?

💬 ryanofsky commented on pull request "Support JSON-RPC 2.0 when requested by client":
(https://github.com/bitcoin/bitcoin/pull/27101#discussion_r1414516901)
In commit "rpc: refactor single/batch requests" (125ef7a61885d2ffc0d32b66cdfb3dced2a4417b)

This change doesn't seem right because `jreq.parse` was just called above on line 196, and now the request is going to be parsed for a second time inside `JSONRPCExecOne` for no reason. I think this could be fixed by just dropping the `jreq.parse` call inside `JSONRPCExecOne`. It looks like later commit 4f22286c4aa65a113c44153847c857075a4225d6 does this anyway, so would be better to drop it up front and
...

💬 achow101 commented on pull request "wallet: batch all individual spkms setup db writes in a single db txn":
(https://github.com/bitcoin/bitcoin/pull/28894#issuecomment-1839550503)
ACK f05302427386fe63f4929a7198652cb1e4ab3bcc

👍 ryanofsky approved a pull request: "script, assumeutxo: Enhance validations in utxo_snapshot.sh"
(https://github.com/bitcoin/bitcoin/pull/28852#pullrequestreview-1763521007)
Code review ACK 11b7269d83a56f919f9dddb7f7c72a96d428627f

💬 ryanofsky commented on pull request "script, assumeutxo: Enhance validations in utxo_snapshot.sh":
(https://github.com/bitcoin/bitcoin/pull/28852#discussion_r1414558799)
In commit "script: Enhance validations in utxo_snapshot.sh" (11b7269d83a56f919f9dddb7f7c72a96d428627f)

Assignment seems a little out of place here, would suggest moving it below `PRUNE=...` with other variable assignments.

✅ ryanofsky closed an issue: "Stuck chainstate when utxo_snapshot.sh fails"
(https://github.com/bitcoin/bitcoin/issues/27841)

🚀 ryanofsky merged a pull request: "script, assumeutxo: Enhance validations in utxo_snapshot.sh"
(https://github.com/bitcoin/bitcoin/pull/28852)

💬 hebasto commented on issue "`capnp` fails when cross-compiling":
(https://github.com/bitcoin/bitcoin/issues/28993#issuecomment-1839567229)
I fail to reproduce the bug in clean docker environment. Perhaps, my machine has a broken environment.

Closing.

✅ hebasto closed an issue: "`capnp` fails when cross-compiling"
(https://github.com/bitcoin/bitcoin/issues/28993)

💬 ryanofsky commented on pull request "script, assumeutxo: Enhance validations in utxo_snapshot.sh":
(https://github.com/bitcoin/bitcoin/pull/28852#issuecomment-1839567625)
Note: this actually had 3 acks. DrahtBot didn't count https://github.com/bitcoin/bitcoin/pull/28852#pullrequestreview-1741060818 due to capitalization

💬 ryanofsky commented on issue "`capnp` fails when cross-compiling":
(https://github.com/bitcoin/bitcoin/issues/28993#issuecomment-1839570160)
That makes more sense. Thanks for the bug report in any case!

💬 furszy commented on pull request "rpc: encryptwallet help, mention HD seed rotation and backup requirement":
(https://github.com/bitcoin/bitcoin/pull/28980#discussion_r1414574791)
> "Therefore, it is crucial to make a fresh backup of the newly generated wallet file."

The goal of the "replace" wording is to avoid situations where users restore the wallet from the previous backup, thinking it is the latest one, and blames core for not detecting the balance.

💬 jonatack commented on pull request "rpc: encryptwallet help, mention HD seed rotation and backup requirement":
(https://github.com/bitcoin/bitcoin/pull/28980#discussion_r1414581640)
> and blames core for not detecting the balance

I think I would avoid recommending that users delete their wallet files, as potential loss of funds is a worse case.

💬 furszy commented on pull request "rpc: encryptwallet help, mention HD seed rotation and backup requirement":
(https://github.com/bitcoin/bitcoin/pull/28980#discussion_r1414582511)
> using the `backupwallet` RPC.

I think the current text is slightly better because it alerts users without scaring them.
But, if there is a higher preference for it, could change it.

💬 furszy commented on pull request "rpc: encryptwallet help, mention HD seed rotation and backup requirement":
(https://github.com/bitcoin/bitcoin/pull/28980#discussion_r1414585672)
Sure. Done as suggested

💬 furszy commented on pull request "rpc: encryptwallet help, mention HD seed rotation and backup requirement":
(https://github.com/bitcoin/bitcoin/pull/28980#issuecomment-1839609896)
Updated per feedback. Thanks everyone.

🤔 ryanofsky reviewed a pull request: "doc: Add multiprocess design doc"
(https://github.com/bitcoin/bitcoin/pull/28978#pullrequestreview-1763577959)
Updated f566245147003648099f961306be82ea32ea47ae -> 837c53d14f24924cdcb2cfd8b18915882dc3b620 ([`pr/ipcdoc.7`](https://github.com/ryanofsky/bitcoin/commits/pr/ipcdoc.7) -> [`pr/ipcdoc.8`](https://github.com/ryanofsky/bitcoin/commits/pr/ipcdoc.8), [compare](https://github.com/ryanofsky/bitcoin/compare/pr/ipcdoc.7..pr/ipcdoc.8)) adding suggested future enhancement and making a few other tweaks

💬 ryanofsky commented on pull request "doc: Add multiprocess design doc":
(https://github.com/bitcoin/bitcoin/pull/28978#discussion_r1414593514)
re: https://github.com/bitcoin/bitcoin/pull/28978#discussion_r1413316233

> * Generating server / client code in different langages (e.g C++ -> Rust or Rust -> C++) from a common set of `capnp` files and with compatible interfaces.

Yes, this significant and worth mentioning. Added some information about this and a link the work you mentioned.