💬 jonatack commented on pull request "script: add description for the functionality of each opcode":
(https://github.com/bitcoin/bitcoin/pull/27109#discussion_r1108829784)
```suggestion
OP_2DUP = 0x6e, // duplicate top and second from top stack items
```
(https://github.com/bitcoin/bitcoin/pull/27109#discussion_r1108829784)
```suggestion
OP_2DUP = 0x6e, // duplicate top and second from top stack items
```
💬 jonatack commented on pull request "script: add description for the functionality of each opcode":
(https://github.com/bitcoin/bitcoin/pull/27109#discussion_r1108829461)
```suggestion
OP_3DUP = 0x6f, // duplicate top, second from top and third from top stack items
```
(https://github.com/bitcoin/bitcoin/pull/27109#discussion_r1108829461)
```suggestion
OP_3DUP = 0x6f, // duplicate top, second from top and third from top stack items
```
💬 jonatack commented on pull request "script: add description for the functionality of each opcode":
(https://github.com/bitcoin/bitcoin/pull/27109#discussion_r1108833548)
```suggestion
OP_NEGATE = 0x8f, // multiply the top stack item by -1
```
(https://github.com/bitcoin/bitcoin/pull/27109#discussion_r1108833548)
```suggestion
OP_NEGATE = 0x8f, // multiply the top stack item by -1
```
💬 martinus commented on pull request "clang-tidy: Add more `performance-*` checks and related fixes":
(https://github.com/bitcoin/bitcoin/pull/26642#discussion_r1108845830)
Good call, I find the `noexcept` stuff so brittle it's really hard to use. E.g. my changes to nanobench.h too were not completely correct because there's no guarantee that `std::string` move assignment is noexcept, see here: https://github.com/martinus/nanobench/pull/87
(https://github.com/bitcoin/bitcoin/pull/26642#discussion_r1108845830)
Good call, I find the `noexcept` stuff so brittle it's really hard to use. E.g. my changes to nanobench.h too were not completely correct because there's no guarantee that `std::string` move assignment is noexcept, see here: https://github.com/martinus/nanobench/pull/87
💬 RandyMcMillan commented on pull request "script: add description for the functionality of each opcode":
(https://github.com/bitcoin/bitcoin/pull/27109#issuecomment-1433504037)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/27109#issuecomment-1433504037)
Concept ACK
💬 kristapsk commented on pull request "script: add description for the functionality of each opcode":
(https://github.com/bitcoin/bitcoin/pull/27109#issuecomment-1433509860)
Concept ACK
(https://github.com/bitcoin/bitcoin/pull/27109#issuecomment-1433509860)
Concept ACK
💬 sipa commented on pull request "script: add description for the functionality of each opcode":
(https://github.com/bitcoin/bitcoin/pull/27109#discussion_r1108865326)
It pushes `"\x81"` (the byte array consisting of a single byte with value 129).
(https://github.com/bitcoin/bitcoin/pull/27109#discussion_r1108865326)
It pushes `"\x81"` (the byte array consisting of a single byte with value 129).
💬 sipa commented on pull request "script: add description for the functionality of each opcode":
(https://github.com/bitcoin/bitcoin/pull/27109#discussion_r1108865017)
I'd just say `""` or `the empty array`. "\x" isn't any valid syntax I know.
(https://github.com/bitcoin/bitcoin/pull/27109#discussion_r1108865017)
I'd just say `""` or `the empty array`. "\x" isn't any valid syntax I know.
👍 pinheadmz approved a pull request: "net: remove orphaned CSubNet::SanityCheck()"
(https://github.com/bitcoin/bitcoin/pull/27106)
(https://github.com/bitcoin/bitcoin/pull/27106)
👍 pinheadmz approved a pull request: "net: remove orphaned CSubNet::SanityCheck()"
(https://github.com/bitcoin/bitcoin/pull/27106)
ACK 30a3230e86dfd49c771432be6219841df5066eb4
<details><summary>Show Signature</summary>
```
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256
ACK 30a3230e86dfd49c771432be6219841df5066eb4
-----BEGIN PGP SIGNATURE-----
iQIzBAEBCAAdFiEE5hdzzW4BBA4vG9eM5+KYS2KJyToFAmPudFYACgkQ5+KYS2KJ
yTrYnA/+LcTOT8/zvgcEeQ/nMhbO3LijgTmdio0OZS/dMc6SKdeAT/mDXv0ThjhB
+K7FuHY9fYO9sbXqPNkrbEfGR3iwqCNu5ElEAmvOMqVZIfkLHy//AX/uKNlUx2h7
1bICzt3PnnhaeGUWWxUeU+HFSpLikl6OHUYZLJfT+2VTz8rdMxhSY5iKSUEZNDKZ
J3D3B5nM
...
(https://github.com/bitcoin/bitcoin/pull/27106)
ACK 30a3230e86dfd49c771432be6219841df5066eb4
<details><summary>Show Signature</summary>
```
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256
ACK 30a3230e86dfd49c771432be6219841df5066eb4
-----BEGIN PGP SIGNATURE-----
iQIzBAEBCAAdFiEE5hdzzW4BBA4vG9eM5+KYS2KJyToFAmPudFYACgkQ5+KYS2KJ
yTrYnA/+LcTOT8/zvgcEeQ/nMhbO3LijgTmdio0OZS/dMc6SKdeAT/mDXv0ThjhB
+K7FuHY9fYO9sbXqPNkrbEfGR3iwqCNu5ElEAmvOMqVZIfkLHy//AX/uKNlUx2h7
1bICzt3PnnhaeGUWWxUeU+HFSpLikl6OHUYZLJfT+2VTz8rdMxhSY5iKSUEZNDKZ
J3D3B5nM
...
💬 jonatack commented on pull request "Add pool based memory resource":
(https://github.com/bitcoin/bitcoin/pull/25325#issuecomment-1433537441)
re-ACK b5eba9ad001f1035dd641bc5880cb6bb53a8b07f
(https://github.com/bitcoin/bitcoin/pull/25325#issuecomment-1433537441)
re-ACK b5eba9ad001f1035dd641bc5880cb6bb53a8b07f
💬 achow101 commented on pull request "wallet: be able to specify a wallet name and passphrase to migratewallet":
(https://github.com/bitcoin/bitcoin/pull/26595#discussion_r1108881969)
I've updated the passphrase handling to match what #27068 does
(https://github.com/bitcoin/bitcoin/pull/26595#discussion_r1108881969)
I've updated the passphrase handling to match what #27068 does
💬 achow101 commented on pull request "wallet: be able to specify a wallet name and passphrase to migratewallet":
(https://github.com/bitcoin/bitcoin/pull/26595#discussion_r1108883010)
> It would be useful to also test the behavior of the wallet encryption post-migration (eg. that migratewallet re-locks the wallet).
Added a check for that.
> A test for when the passphrase is provided but is incorrect could also be added here.
Done
> A test for when the wallet name is provided but is incorrect could also be added.
There's a test for that already in `test_unloaded`.
(https://github.com/bitcoin/bitcoin/pull/26595#discussion_r1108883010)
> It would be useful to also test the behavior of the wallet encryption post-migration (eg. that migratewallet re-locks the wallet).
Added a check for that.
> A test for when the passphrase is provided but is incorrect could also be added here.
Done
> A test for when the wallet name is provided but is incorrect could also be added.
There's a test for that already in `test_unloaded`.
💬 achow101 commented on pull request "wallet: be able to specify a wallet name and passphrase to migratewallet":
(https://github.com/bitcoin/bitcoin/pull/26595#discussion_r1108884105)
Fixed
(https://github.com/bitcoin/bitcoin/pull/26595#discussion_r1108884105)
Fixed
💬 achow101 commented on pull request "wallet: be able to specify a wallet name and passphrase to migratewallet":
(https://github.com/bitcoin/bitcoin/pull/26595#issuecomment-1433550228)
> > Wallet file verification failed. Failed to load database path '/tmp/bitcoin_func_test_w3iehdpt/node0/regtest/wallets/wrong_wallet_name'. Path does not exist. (-4)
>
> It would make more sense to throw a simpler error from `migratewallet` (as opposed to from `MakeDatabase`).
This is the same error that we currently give for `loadwallet`. I'm going to leave it as is so these are consistent with each other, but I agree it could be improved.
(https://github.com/bitcoin/bitcoin/pull/26595#issuecomment-1433550228)
> > Wallet file verification failed. Failed to load database path '/tmp/bitcoin_func_test_w3iehdpt/node0/regtest/wallets/wrong_wallet_name'. Path does not exist. (-4)
>
> It would make more sense to throw a simpler error from `migratewallet` (as opposed to from `MakeDatabase`).
This is the same error that we currently give for `loadwallet`. I'm going to leave it as is so these are consistent with each other, but I agree it could be improved.
🚀 achow101 merged a pull request: "New `outputs` argument for `bumpfee`/`psbtbumpfee`"
(https://github.com/bitcoin/bitcoin/pull/25344)
(https://github.com/bitcoin/bitcoin/pull/25344)
💬 ryanofsky commented on pull request "refactor: Add util::Result failure values, multiple error and warning messages":
(https://github.com/bitcoin/bitcoin/pull/25665#issuecomment-1433568160)
re: https://github.com/bitcoin/bitcoin/pull/25665#pullrequestreview-1294756069
Thanks for the review! I got rid of the unused test file and changed the `if` formatting as suggested
---
Updated eb50fcd6859d1730663159995e8477f6d892e7f4 -> 501ef88b9412b0d924abf32cf2de7fbcbbb69b8d ([`pr/bresult2.28`](https://github.com/ryanofsky/bitcoin/commits/pr/bresult2.28) -> [`pr/bresult2.29`](https://github.com/ryanofsky/bitcoin/commits/pr/bresult2.29), [compare](https://github.com/ryanofsky/bitcoin/c
...
(https://github.com/bitcoin/bitcoin/pull/25665#issuecomment-1433568160)
re: https://github.com/bitcoin/bitcoin/pull/25665#pullrequestreview-1294756069
Thanks for the review! I got rid of the unused test file and changed the `if` formatting as suggested
---
Updated eb50fcd6859d1730663159995e8477f6d892e7f4 -> 501ef88b9412b0d924abf32cf2de7fbcbbb69b8d ([`pr/bresult2.28`](https://github.com/ryanofsky/bitcoin/commits/pr/bresult2.28) -> [`pr/bresult2.29`](https://github.com/ryanofsky/bitcoin/commits/pr/bresult2.29), [compare](https://github.com/ryanofsky/bitcoin/c
...
💬 1440000bytes commented on pull request "New `outputs` argument for `bumpfee`/`psbtbumpfee`":
(https://github.com/bitcoin/bitcoin/pull/25344#issuecomment-1433572881)
Finally. Thanks @rodentrabies this would improve UX.
(https://github.com/bitcoin/bitcoin/pull/25344#issuecomment-1433572881)
Finally. Thanks @rodentrabies this would improve UX.
💬 martinus commented on pull request "clang-tidy: Add more `performance-*` checks and related fixes":
(https://github.com/bitcoin/bitcoin/pull/26642#discussion_r1108890317)
In `BOOST_AUTO_TEST_CASE(set)` there's another `std::vector<CPubKey> keys;` without reserve, I wonder why clang-tidy doesn't see these
(https://github.com/bitcoin/bitcoin/pull/26642#discussion_r1108890317)
In `BOOST_AUTO_TEST_CASE(set)` there's another `std::vector<CPubKey> keys;` without reserve, I wonder why clang-tidy doesn't see these
💬 martinus commented on pull request "clang-tidy: Add more `performance-*` checks and related fixes":
(https://github.com/bitcoin/bitcoin/pull/26642#discussion_r1108879800)
nit: could be 10, 5 more are added :)
(https://github.com/bitcoin/bitcoin/pull/26642#discussion_r1108879800)
nit: could be 10, 5 more are added :)