💬 itornaza commented on pull request "tests: fix `OP_1NEGATE` handling in `CScriptOp`":
(https://github.com/bitcoin/bitcoin/pull/29589#issuecomment-2052160310)
tested ACK f0c077a388b980566590d7887d8c6dd97cbaa20a

- Performed a code review and everything lgtm
- Configured with --with-incompatible-bdb and --enable-suppress-external-warnings
- Built the PR, ran unit tests with make check and all tests pass
- Ran all functional tests with no extra flags and all tests pass
- Ran all functional tests with --extended and all tests pass

💬 dgpv commented on pull request "tests: fix `OP_1NEGATE` handling in `CScriptOp`":
(https://github.com/bitcoin/bitcoin/pull/29589#issuecomment-2052181600)
NACK for changing behavior of `decode_op_n()` and `encode_op_n()`

(the reasoning is given above, in https://github.com/bitcoin/bitcoin/pull/29589#issuecomment-1996854762)

💬 pablomartin4btc commented on pull request "Improve 'Requested Payments History' Multiselect":
(https://github.com/bitcoin-core/gui/pull/684#issuecomment-2052272585)
I could take this to move it forward @john-moffett, if you are not able to, please let me know.

👍 byaye approved a pull request: "doc: i2p: improve `-i2pacceptincoming` mention"
(https://github.com/bitcoin/bitcoin/pull/29813#pullrequestreview-1998244687)
ACK 2179e2c3209a41c1419f1f5ed6270a0dad68b50d

👍 1440000bytes approved a pull request: "net: Decrease nMaxIPs when learning from DNS seeds"
(https://github.com/bitcoin/bitcoin/pull/29850#pullrequestreview-1998246704)
utACK https://github.com/bitcoin/bitcoin/pull/29850/commits/f2e3662e57eca1330962faf38ff428a564d50a11

💬 1440000bytes commented on pull request "policy: Allow non-standard scripts with -acceptnonstdtxn=1 (test nets only)":
(https://github.com/bitcoin/bitcoin/pull/29843#issuecomment-2052349369)
Concept ACK

💬 pablomartin4btc commented on pull request "Add new "address type" column to the "receiving tab" address book page":
(https://github.com/bitcoin-core/gui/pull/753#discussion_r1563067060)
Good point, this is something that could be subject for discussion, I'm open to any suggestions. Originally, as for this change, I decided to change the resize mode from `Stretch` to `ResizeToContents` as it looked tighter to me (resize mode [enum](https://doc.qt.io/qt-5/qheaderview.html#ResizeMode-enum) - qt doc ref). I was thinking to make a bit of a mix of both of them, `Stretch` for some columns and `ResizeToContents` to others (e.g. check [here](https://forum.qt.io/topic/115561/qheaderview-
...

💬 benthecarman commented on pull request "policy: Allow non-standard scripts with -acceptnonstdtxn=1 (test nets only)":
(https://github.com/bitcoin/bitcoin/pull/29843#issuecomment-2052415425)
Concept ACK this would be nice for testing soft forks on test networks

👍 byaye approved a pull request: "assumeutxo: Fix -reindex before snapshot was validated"
(https://github.com/bitcoin/bitcoin/pull/29726#pullrequestreview-1998392605)
Tested ACK b7ba60f81a33db876f88b5f9af1e5025d679b5be

Test result before the fix
<img width="837" alt="Screenshot 2024-04-12 at 17 34 14" src="https://github.com/bitcoin/bitcoin/assets/19962379/1449dd8e-dd6e-466e-b34c-b49d6a04c343">

Test result after the fix
<img width="856" alt="Screenshot 2024-04-12 at 17 21 48" src="https://github.com/bitcoin/bitcoin/assets/19962379/fe76c207-1b76-4cb9-91f7-8d3626e029de">

💬 okorye commented on issue "Error unlocking wallet: some keys decrypt but not all. your wallet file may be corrupt. ":
(https://github.com/bitcoin/bitcoin/issues/29789#issuecomment-2052612192)
> ### Is there an existing issue for this?
>
> - [X] I have searched the existing issues
>
> ### Current behaviour
>
> My personal computer crashed and I've already imported "wallet.dat" in the latest Bitcoin Core v26.0.0. It takes time to synchronize the wallet that was about 6 days.
>
> Anyhow, now my wallet is updated and I can see the correct balance of my Bitcoin in the "Overview section". I have all the needed information which I took as a backup such as "passphrases" and "wallet.dat
...

👍 hernanmarino approved a pull request: "test: fix accurate multisig sigop count (BIP16), add unit test"
(https://github.com/bitcoin/bitcoin/pull/29615#pullrequestreview-1998510374)
tACK 3e9c736a26724ffe3b70b387995fbf48c06300e2

💬 hernanmarino commented on pull request "refactor: Avoid unsigned integer overflow in `script/interpreter.cpp`":
(https://github.com/bitcoin/bitcoin/pull/29543#issuecomment-2052640557)
ACK to the idea of removing overflows in the code, but I was little worried of the implications of this. After reading @sipa 's comment, it is definitely an ACK for me. Regarding the approach between this PR and https://github.com/bitcoin/bitcoin/pull/24214 , I am leaning towards this for the sole reason that 24214 is still closed, but i reviewed and also ACK that PR as well.

💬 hernanmarino commented on pull request "Fix unsigned integer overflows in interpreter":
(https://github.com/bitcoin/bitcoin/pull/24214#issuecomment-2052642229)
cr ACK in case this gets reopened, instead of the alternative https://github.com/bitcoin/bitcoin/pull/29543

🤔 hernanmarino reviewed a pull request: "policy: bump TX_MAX_STANDARD_VERSION to 3"
(https://github.com/bitcoin/bitcoin/pull/29496#pullrequestreview-1998604005)
Concept ACK 7b18f0ed94557a93c69b4f19d85c4fd6fd480464

🤔 hernanmarino reviewed a pull request: "Drop log category in `SeedStartup`"
(https://github.com/bitcoin/bitcoin/pull/29480#pullrequestreview-1998611427)
ACK 88468a8afcd908cd26f4e6ecfb86c80d1c56fe73

🤔 hernanmarino reviewed a pull request: "test: fix RPC coverage check"
(https://github.com/bitcoin/bitcoin/pull/29387#pullrequestreview-1998647499)
Concept ACK , but still a little concerned about @maflcko last comment ( https://github.com/bitcoin/bitcoin/pull/29387#issuecomment-1959076888 )

🤔 hernanmarino reviewed a pull request: "refactor: Allow CScript construction from any std::input_iterator"
(https://github.com/bitcoin/bitcoin/pull/29369#pullrequestreview-1998653786)
utACK fa40ae2dbd181b27371b91636652c9b408585384

🤔 hernanmarino reviewed a pull request: "doc: fixup help output for -upnp and -natpmp"
(https://github.com/bitcoin/bitcoin/pull/28874#pullrequestreview-1998681170)
ACK 92f88a962908c49dde99c03a4608e63e4a6eec71

💬 hernanmarino commented on pull request "Don't empty dbcache on prune flushes: >30% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/28280#issuecomment-2052697672)
Concept ACK . I 'd really like to see this merged

👍 hernanmarino approved a pull request: "validation: don't clear cache on periodic flush: >2x block connection speed"
(https://github.com/bitcoin/bitcoin/pull/28233#pullrequestreview-1998740372)
Concept ACK. I also agree with the criteria in the code for setting the `empty_cache` variable to true