💬 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.

🚀 achow101 merged a pull request: "New `outputs` argument for `bumpfee`/`psbtbumpfee`"
(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
...

💬 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.

💬 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

💬 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 :)

💬 ajtowns commented on pull request "Support JSON-RPC 2.0 when requested by client":
(https://github.com/bitcoin/bitcoin/pull/27101#discussion_r1106907468)
Rather than `bool is20`, `enum JSONVersion { JSON_1_BTC, JSON_2_0 }` might be clearer?

💬 RandyMcMillan commented on pull request "script: add description for the functionality of each opcode":
(https://github.com/bitcoin/bitcoin/pull/27109#issuecomment-1433649698)
applying clang-formatting:
per `src/.clang-format`
may be useful

https://github.com/RandyMcMillan/bitcoin/commit/99e3dc536dca94dffd703e77572f6dca91f4bb26

📝 MarcoFalke converted_to_draft a pull request: "ci: Add copyright header check"
(https://github.com/bitcoin/bitcoin/pull/27100)
Currently some files have a copyright header and some don't. There is no check for this.

Not sure for which files we want to enforce copyright headers, if any at all. However if we do it for any files, it should be enforced by CI. See also the section "Copyright" in https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md#copyright

So implement a check for this and add a config file to the `test/lint/` directory.

Can be tested by emptying the config file and calling `(cd test/lint
...

📝 brunoerg opened a pull request: "p2p: Allow whitelisting outgoing connections"
(https://github.com/bitcoin/bitcoin/pull/27114)
Revives #17167. It allows whitelisting outgoing connections.

I previously revived it with #26441, however, some reviewers told me it's better to keep them splitted to facilitate reviews and probably having this one merged first since we need it to improve functional tests.

💬 brunoerg commented on pull request "p2p: Allow whitelisting outgoing connections":
(https://github.com/bitcoin/bitcoin/pull/27114#issuecomment-1433677572)
cc: @furszy @theStack

📝 brunoerg converted_to_draft a pull request: "rpc, p2p: add `addpermissionflags` RPC and allow whitelisting outbound"
(https://github.com/bitcoin/bitcoin/pull/26441)
Built this PR on top of #17167 (that's been closed due to inactivity but had some Concept ACK). So, it allows whitelisting outbound peers.

This PR adds a new RPC `addpermissionflags` to be able to set up permission flags -`whitelist` thru RPC, so we don't need to restart our node if we want to add new flags.

E.g.
```sh
$ ./src/bitcoin-cli addpermissionflags ["noban", "mempool", "in", "out"] "127.0.0.1"
```

💬 brunoerg commented on pull request "rpc, p2p: add `addpermissionflags` RPC and allow whitelisting outbound":
(https://github.com/bitcoin/bitcoin/pull/26441#issuecomment-1433679228)
Converted it to draft to address suggestions and re-build it on top of #27114.

💬 brunoerg commented on pull request "rpc, p2p: add `addpermissionflags` RPC and allow whitelisting outbound":
(https://github.com/bitcoin/bitcoin/pull/26441#discussion_r1108995297)
Done in #27114

💬 brunoerg commented on pull request "rpc, p2p: add `addpermissionflags` RPC and allow whitelisting outbound":
(https://github.com/bitcoin/bitcoin/pull/26441#discussion_r1108995480)
Done in #27114

💬 MarcoFalke commented on pull request "doc: Bump copyright years to present (headers only)":
(https://github.com/bitcoin/bitcoin/pull/26817#issuecomment-1433685289)
Adjusted title, description and commits according to IRC discussion and comment https://github.com/bitcoin/bitcoin/pull/26817#issuecomment-1400897999

💬 willcl-ark commented on pull request "Convert ArgsManager::GetDataDir to a read-only function":
(https://github.com/bitcoin/bitcoin/pull/27073#discussion_r1109015893)
The reason is that we do not test relative `-conf=` arguments [here](https://github.com/bitcoin/bitcoin/blob/73966f75f67fb797163f0a766292a79d4b2c1b70/test/functional/feature_config_args.py#L287), even though we pass a realtive conf argument in.

This means that we are only testing the `os.path.join` sum of default `DataDir` and the relative arg.

💬 john-moffett commented on pull request "script: add description for the functionality of each opcode":
(https://github.com/bitcoin/bitcoin/pull/27109#discussion_r1109011531)
We should probably be consistent in describing how Script deals with true/false, since it's later described as `if top stack value != 0`. Maybe add a section along the lines of:

```
/**
* Opcodes that take a true/false value will evaluate the following as false:
* an empty vector
* a vector (of any length) of all zero bytes
* a single byte of "\x80" ('negative zero')
* a vector (of any length) of all zero bytes except the last byte is "\x80"
*
* Any other value wil
...

💬 willcl-ark commented on issue "Setting `onlynet=onion` still makes some IPv4 connections.":
(https://github.com/bitcoin/bitcoin/issues/12344#issuecomment-1433731288)
@Willtech I see from your User Agent you are using v0.20.1 (now outside of maintainance I think too FYI), are you able to try with v24.0.1 to see if the problem still occurs?

👍 brunoerg approved a pull request: "net: remove orphaned CSubNet::SanityCheck()"
(https://github.com/bitcoin/bitcoin/pull/27106)
crACK 30a3230e86dfd49c771432be6219841df5066eb4

💬 Xekyo commented on pull request "Implement Mini version of BlockAssembler to calculate mining scores":
(https://github.com/bitcoin/bitcoin/pull/27021#issuecomment-1433738562)
Awesome, thanks for the reworking this, @glozow, and the work on the fuzzer, @dergoegge. I've fixed a minor `tidy` issue and I’ll pick the chain interface changes into #26152 and rebase on this shortly.

Ready for review