💬 maflcko commented on pull request "rename TransactionErrors: MISSING_INPUTS and ALREADY_IN_CHAIN":
(https://github.com/bitcoin/bitcoin/pull/30212#discussion_r1663937723)
I also don't understand why the first commit is needed. The error message is already correct (and unchanged in this pull request): `Untranslated("Inputs missing or spent")`.
I'd say it is fine to drop the commit.
(https://github.com/bitcoin/bitcoin/pull/30212#discussion_r1663937723)
I also don't understand why the first commit is needed. The error message is already correct (and unchanged in this pull request): `Untranslated("Inputs missing or spent")`.
I'd say it is fine to drop the commit.
💬 willcl-ark commented on pull request "Ignore files ignored by git in the Markdown Link Checker lint":
(https://github.com/bitcoin/bitcoin/pull/30380#discussion_r1663949766)
Yes, it looks like it can. Added a commit for that
(https://github.com/bitcoin/bitcoin/pull/30380#discussion_r1663949766)
Yes, it looks like it can. Added a commit for that
💬 maflcko commented on pull request "psbt: Check non witness utxo outpoint early":
(https://github.com/bitcoin/bitcoin/pull/29855#issuecomment-2205695176)
It would be nice to have this in 28.x
(https://github.com/bitcoin/bitcoin/pull/29855#issuecomment-2205695176)
It would be nice to have this in 28.x
💬 willcl-ark commented on pull request "Fix cases of calls to `FillPSBT` errantly returning `complete=true`":
(https://github.com/bitcoin/bitcoin/pull/30357#discussion_r1663950622)
Done in 7e36dca657c66bc70b04d5b850e5a335aecfb902
(https://github.com/bitcoin/bitcoin/pull/30357#discussion_r1663950622)
Done in 7e36dca657c66bc70b04d5b850e5a335aecfb902
👍 maflcko approved a pull request: "lint: Ignore files ignored by git in the Markdown Link Checker"
(https://github.com/bitcoin/bitcoin/pull/30380#pullrequestreview-2156055054)
lgtm
(https://github.com/bitcoin/bitcoin/pull/30380#pullrequestreview-2156055054)
lgtm
📝 willcl-ark opened a pull request: "[WIP] net: return result from addnode RPC"
(https://github.com/bitcoin/bitcoin/pull/30381)
Fixes: #20552
Picks up a [branch](https://github.com/bitcoin/bitcoin/compare/master...amitiuttarwar:bitcoin:2020-11-open-conn-improvements) from by @amitiuttarwar which adds a success response to `addnode` RPC.
It adds a new return object to `addnode` RPC indicating that the call was successful:
```cpp
{
{RPCResult::Type::STR, "operation", "The operation called (onetry/add/remove)"},
{RPCResult::Type::STR, "result", "The result of the operation (success/failed)"},
{RPCRe
...
(https://github.com/bitcoin/bitcoin/pull/30381)
Fixes: #20552
Picks up a [branch](https://github.com/bitcoin/bitcoin/compare/master...amitiuttarwar:bitcoin:2020-11-open-conn-improvements) from by @amitiuttarwar which adds a success response to `addnode` RPC.
It adds a new return object to `addnode` RPC indicating that the call was successful:
```cpp
{
{RPCResult::Type::STR, "operation", "The operation called (onetry/add/remove)"},
{RPCResult::Type::STR, "result", "The result of the operation (success/failed)"},
{RPCRe
...
💬 willcl-ark commented on pull request "[WIP] net: return result from addnode RPC":
(https://github.com/bitcoin/bitcoin/pull/30381#issuecomment-2205725737)
I did experiment with not throwing `JSONRPCError`s at all from this call, and always returning a `result` and optionally an `error` if it occurred, but this doesn't align with our general practice of throwing `JSONRPCError`s on failures, so I abandoned this approach.
This does mean that the `result` field here is pretty much redundant; we either throw, or return a result (implicitly indidcating success). Perhaps it could therefore be removed? Unsure if folks would prefer the re-assurance of s
...
(https://github.com/bitcoin/bitcoin/pull/30381#issuecomment-2205725737)
I did experiment with not throwing `JSONRPCError`s at all from this call, and always returning a `result` and optionally an `error` if it occurred, but this doesn't align with our general practice of throwing `JSONRPCError`s on failures, so I abandoned this approach.
This does mean that the `result` field here is pretty much redundant; we either throw, or return a result (implicitly indidcating success). Perhaps it could therefore be removed? Unsure if folks would prefer the re-assurance of s
...
💬 willcl-ark commented on pull request "[WIP] net: return result from addnode RPC":
(https://github.com/bitcoin/bitcoin/pull/30381#issuecomment-2205734826)
Before these changes:
```log
$ src/bitcoin-cli -signet addnode i-am-not-an-address onetry
```
After:
```log
$ src/bitcoin-cli -signet addnode i-am-not-an-address onetry
error code: -24
error message:
Unable to open connection
```
(https://github.com/bitcoin/bitcoin/pull/30381#issuecomment-2205734826)
Before these changes:
```log
$ src/bitcoin-cli -signet addnode i-am-not-an-address onetry
```
After:
```log
$ src/bitcoin-cli -signet addnode i-am-not-an-address onetry
error code: -24
error message:
Unable to open connection
```
💬 willcl-ark commented on issue "addnode RPC call should return success/failure indicator":
(https://github.com/bitcoin/bitcoin/issues/20552#issuecomment-2205735199)
Opened #30381 to address this issue.
(https://github.com/bitcoin/bitcoin/issues/20552#issuecomment-2205735199)
Opened #30381 to address this issue.
💬 maflcko commented on pull request "build: Bump clang minimum supported version to 16":
(https://github.com/bitcoin/bitcoin/pull/30263#discussion_r1663965619)
Thanks for checking. I'd say dropping support for macos-13+xcode should be done in a follow-up (probably far into the future).
Or was it meant as a nit to remove the `?` in this line of code? If yes, I'll address it, if I have to retouch or rebase.
(https://github.com/bitcoin/bitcoin/pull/30263#discussion_r1663965619)
Thanks for checking. I'd say dropping support for macos-13+xcode should be done in a follow-up (probably far into the future).
Or was it meant as a nit to remove the `?` in this line of code? If yes, I'll address it, if I have to retouch or rebase.
💬 ryanofsky commented on pull request "kernel: De-globalize validation caches":
(https://github.com/bitcoin/bitcoin/pull/30141#issuecomment-2205758873)
> I don't think this is related to `range-diff`. It is simply a typo, which can be missed by a human reviewer regardless of how the code or diff is displayed. (I didn't use range-diff and missed it, fwiw)
I'll often ACK a PR and make a suggestion like you can "replace foo/2 with bar". Then if the PR is updated, I will check the range diff and reACK with "the only thing that changed since last review is replacing foo/2 with bar" without looking at the complete diff again. This is potentially d
...
(https://github.com/bitcoin/bitcoin/pull/30141#issuecomment-2205758873)
> I don't think this is related to `range-diff`. It is simply a typo, which can be missed by a human reviewer regardless of how the code or diff is displayed. (I didn't use range-diff and missed it, fwiw)
I'll often ACK a PR and make a suggestion like you can "replace foo/2 with bar". Then if the PR is updated, I will check the range diff and reACK with "the only thing that changed since last review is replacing foo/2 with bar" without looking at the complete diff again. This is potentially d
...
💬 maflcko commented on issue "Use C++20 consteval to verify translation strings":
(https://github.com/bitcoin/bitcoin/issues/30379#issuecomment-2205761015)
```diff
diff --git a/src/util/translation.h b/src/util/translation.h
index d33fd2d0a0..7d82264a43 100644
--- a/src/util/translation.h
+++ b/src/util/translation.h
@@ -71,7 +71,7 @@ const extern std::function<std::string(const char*)> G_TRANSLATION_FUN;
* Translation function.
* If no translation function is set, simply return the input.
*/
-inline bilingual_str _(const char* psz)
+inline consteval bilingual_str _(const char* psz)
{
return bilingual_str{psz, G_TRANSLATION_
...
(https://github.com/bitcoin/bitcoin/issues/30379#issuecomment-2205761015)
```diff
diff --git a/src/util/translation.h b/src/util/translation.h
index d33fd2d0a0..7d82264a43 100644
--- a/src/util/translation.h
+++ b/src/util/translation.h
@@ -71,7 +71,7 @@ const extern std::function<std::string(const char*)> G_TRANSLATION_FUN;
* Translation function.
* If no translation function is set, simply return the input.
*/
-inline bilingual_str _(const char* psz)
+inline consteval bilingual_str _(const char* psz)
{
return bilingual_str{psz, G_TRANSLATION_
...
🚀 fanquake merged a pull request: "lint: Ignore files ignored by git in the Markdown Link Checker"
(https://github.com/bitcoin/bitcoin/pull/30380)
(https://github.com/bitcoin/bitcoin/pull/30380)
✅ maflcko closed an issue: "Use C++20 consteval to verify translation strings"
(https://github.com/bitcoin/bitcoin/issues/30379)
(https://github.com/bitcoin/bitcoin/issues/30379)
💬 maflcko commented on issue "Use C++20 consteval to verify translation strings":
(https://github.com/bitcoin/bitcoin/issues/30379#issuecomment-2205773613)
nvm, I'll open a pull request soon.
(https://github.com/bitcoin/bitcoin/issues/30379#issuecomment-2205773613)
nvm, I'll open a pull request soon.
⚠️ dg123lzk opened an issue: "I want to set up a node quickly, do you have a snapshot?"
(https://github.com/bitcoin/bitcoin/issues/30382)
I want to set up a node quickly, do you have a snapshot?
(https://github.com/bitcoin/bitcoin/issues/30382)
I want to set up a node quickly, do you have a snapshot?
✅ willcl-ark closed an issue: "I want to set up a node quickly, do you have a snapshot?"
(https://github.com/bitcoin/bitcoin/issues/30382)
(https://github.com/bitcoin/bitcoin/issues/30382)
💬 willcl-ark commented on issue "I want to set up a node quickly, do you have a snapshot?":
(https://github.com/bitcoin/bitcoin/issues/30382#issuecomment-2205785013)
Hi @dg123lzk
General bitcoin questions and/or support requests are best directed to the [Bitcoin StackExchange](https://bitcoin.stackexchange.com/) or the `#bitcoin` IRC channel on the [Libera Chat](https://libera.chat/) network.
For proposed protocol changes you can post to the [bitcoindev mailing list](https://groups.google.com/g/bitcoindev) or the [Delving Bitcoin](https://delvingbitcoin.org/) Discorse forum.
For general bitcoin discussion you can try [bitcointalk](bitcointalk.org)
...
(https://github.com/bitcoin/bitcoin/issues/30382#issuecomment-2205785013)
Hi @dg123lzk
General bitcoin questions and/or support requests are best directed to the [Bitcoin StackExchange](https://bitcoin.stackexchange.com/) or the `#bitcoin` IRC channel on the [Libera Chat](https://libera.chat/) network.
For proposed protocol changes you can post to the [bitcoindev mailing list](https://groups.google.com/g/bitcoindev) or the [Delving Bitcoin](https://delvingbitcoin.org/) Discorse forum.
For general bitcoin discussion you can try [bitcointalk](bitcointalk.org)
...
👍 fanquake approved a pull request: "26.2 final changes"
(https://github.com/bitcoin/bitcoin/pull/30376#pullrequestreview-2156124267)
ACK eef5dbc21b3fd52069f2f0855fb76a13234ebbf3 If you want, you could also backport the changes to get the ASAN job running again, but that isn't blocking here.
(https://github.com/bitcoin/bitcoin/pull/30376#pullrequestreview-2156124267)
ACK eef5dbc21b3fd52069f2f0855fb76a13234ebbf3 If you want, you could also backport the changes to get the ASAN job running again, but that isn't blocking here.
💬 hebasto commented on pull request "build: Bump clang minimum supported version to 16":
(https://github.com/bitcoin/bitcoin/pull/30263#discussion_r1663999366)
It did not mean any changes to the current branch :)
It was just a note (mostly for myself).
(https://github.com/bitcoin/bitcoin/pull/30263#discussion_r1663999366)
It did not mean any changes to the current branch :)
It was just a note (mostly for myself).