💬 maflcko commented on pull request "doc: Improve `dependencies.md`":
(https://github.com/bitcoin/bitcoin/pull/31895#discussion_r2091461356)
I don't any x11 packages are listed here, so it seems unrelated?
(https://github.com/bitcoin/bitcoin/pull/31895#discussion_r2091461356)
I don't any x11 packages are listed here, so it seems unrelated?
🤔 ismaelsadeeq reviewed a pull request: "multiprocess: Add bitcoin wrapper executable"
(https://github.com/bitcoin/bitcoin/pull/31375#pullrequestreview-2843509023)
Code review 7af6e1089ea264e870b26ac83e81e7aa374acbe1
I've also tested this on macOS.
It was running smoothly. I tried shutting it down using the CLI interface, and it worked as expected.
However, I encountered an issue when performing an unclean shutdown using CTRL+C.
The process hung, and I had to `pkill bitcoind` to terminate it.
I wasn’t running with -debug option, so I couldn’t figure out exactly what went wrong. I tried to reproduce the issue but couldn’t It worked smoothly in
...
(https://github.com/bitcoin/bitcoin/pull/31375#pullrequestreview-2843509023)
Code review 7af6e1089ea264e870b26ac83e81e7aa374acbe1
I've also tested this on macOS.
It was running smoothly. I tried shutting it down using the CLI interface, and it worked as expected.
However, I encountered an issue when performing an unclean shutdown using CTRL+C.
The process hung, and I had to `pkill bitcoind` to terminate it.
I wasn’t running with -debug option, so I couldn’t figure out exactly what went wrong. I tried to reproduce the issue but couldn’t It worked smoothly in
...
💬 ismaelsadeeq commented on pull request "multiprocess: Add bitcoin wrapper executable":
(https://github.com/bitcoin/bitcoin/pull/31375#discussion_r2091450477)
In "multiprocess: Add bitcoin wrapper executable" https://github.com/bitcoin/bitcoin/commit/ceabc8cd43fa17be795974a85083aa8e865c654d
help seems redundant here we can get same behavior with -h, maybe just add -a?
(https://github.com/bitcoin/bitcoin/pull/31375#discussion_r2091450477)
In "multiprocess: Add bitcoin wrapper executable" https://github.com/bitcoin/bitcoin/commit/ceabc8cd43fa17be795974a85083aa8e865c654d
help seems redundant here we can get same behavior with -h, maybe just add -a?
💬 ismaelsadeeq commented on pull request "multiprocess: Add bitcoin wrapper executable":
(https://github.com/bitcoin/bitcoin/pull/31375#discussion_r2091062670)
In "multiprocess: Add bitcoin wrapper executable" https://github.com/bitcoin/bitcoin/commit/ceabc8cd43fa17be795974a85083aa8e865c654d
Should the list of executable mentioned that bitcoin-node and bitcoin-gui since are hidden unlike bitcoind and should be accessed via bitcoin process?
(https://github.com/bitcoin/bitcoin/pull/31375#discussion_r2091062670)
In "multiprocess: Add bitcoin wrapper executable" https://github.com/bitcoin/bitcoin/commit/ceabc8cd43fa17be795974a85083aa8e865c654d
Should the list of executable mentioned that bitcoin-node and bitcoin-gui since are hidden unlike bitcoind and should be accessed via bitcoin process?
💬 ismaelsadeeq commented on pull request "multiprocess: Add bitcoin wrapper executable":
(https://github.com/bitcoin/bitcoin/pull/31375#discussion_r2091131888)
Shouldn't we throw when that happen?
(https://github.com/bitcoin/bitcoin/pull/31375#discussion_r2091131888)
Shouldn't we throw when that happen?
💬 ismaelsadeeq commented on pull request "multiprocess: Add bitcoin wrapper executable":
(https://github.com/bitcoin/bitcoin/pull/31375#discussion_r2091060261)
In "multiprocess: Add bitcoin wrapper executable" https://github.com/bitcoin/bitcoin/commit/ceabc8cd43fa17be795974a85083aa8e865c654d
Copy right bump to 2025-present.
(https://github.com/bitcoin/bitcoin/pull/31375#discussion_r2091060261)
In "multiprocess: Add bitcoin wrapper executable" https://github.com/bitcoin/bitcoin/commit/ceabc8cd43fa17be795974a85083aa8e865c654d
Copy right bump to 2025-present.
💬 ismaelsadeeq commented on pull request "multiprocess: Add bitcoin wrapper executable":
(https://github.com/bitcoin/bitcoin/pull/31375#discussion_r2091447427)
In "multiprocess: Add bitcoin wrapper executable" https://github.com/bitcoin/bitcoin/commit/ceabc8cd43fa17be795974a85083aa8e865c654d
I think we should prevent mixing up options with commands using this suggestion or something like it.
If you pass -v you should not pass any other command same with help
```suggestion
if (cmd.show_version) {
if (argc > 2) {
tfm::format(std::cout, "Error: too many arguments");
return EXIT_FAILURE;
...
(https://github.com/bitcoin/bitcoin/pull/31375#discussion_r2091447427)
In "multiprocess: Add bitcoin wrapper executable" https://github.com/bitcoin/bitcoin/commit/ceabc8cd43fa17be795974a85083aa8e865c654d
I think we should prevent mixing up options with commands using this suggestion or something like it.
If you pass -v you should not pass any other command same with help
```suggestion
if (cmd.show_version) {
if (argc > 2) {
tfm::format(std::cout, "Error: too many arguments");
return EXIT_FAILURE;
...
💬 ismaelsadeeq commented on pull request "multiprocess: Add bitcoin wrapper executable":
(https://github.com/bitcoin/bitcoin/pull/31375#discussion_r2091152243)
In "multiprocess: Add bitcoin wrapper executable" ceabc8cd43fa17be795974a85083aa8e865c654d
Are these actually commands?
Maybe *target* would be a better name. The commands are what's being forwarded to the executable, right?
It gets confusing in the error message, like when you pass `cli`:
```
Error: Unrecognized command: 'cli'
```
Maybe it would be clearer as:
```
Error: Unrecognized target: 'cli'
```
It gets more ambigous when you omit the command in the RPC
the
...
(https://github.com/bitcoin/bitcoin/pull/31375#discussion_r2091152243)
In "multiprocess: Add bitcoin wrapper executable" ceabc8cd43fa17be795974a85083aa8e865c654d
Are these actually commands?
Maybe *target* would be a better name. The commands are what's being forwarded to the executable, right?
It gets confusing in the error message, like when you pass `cli`:
```
Error: Unrecognized command: 'cli'
```
Maybe it would be clearer as:
```
Error: Unrecognized target: 'cli'
```
It gets more ambigous when you omit the command in the RPC
the
...
👋 fanquake's pull request is ready for review: "[28.x] 28.2rc1"
(https://github.com/bitcoin/bitcoin/pull/32480)
(https://github.com/bitcoin/bitcoin/pull/32480)
📝 pinheadmz opened a pull request: "rpc: add "ischange: true" to decoded tx outputs in wallet gettransaction response"
(https://github.com/bitcoin/bitcoin/pull/32517)
This change is motivated by external RBF clients like https://github.com/CardCoins/additive-rbf-batcher/. It saves the user a redundant re-looping of tx outputs, calling `getaddressinfo` on each one, looking for the change output in order to adjust the fee.
The field `"ischange"` only appears when `gettransaction` is called on a wallet, and is either `true` or not present at all. I chose not to include `ischange: false` because it is confusing to see that on *received* transactions.
Exampl
...
(https://github.com/bitcoin/bitcoin/pull/32517)
This change is motivated by external RBF clients like https://github.com/CardCoins/additive-rbf-batcher/. It saves the user a redundant re-looping of tx outputs, calling `getaddressinfo` on each one, looking for the change output in order to adjust the fee.
The field `"ischange"` only appears when `gettransaction` is called on a wallet, and is either `true` or not present at all. I chose not to include `ischange: false` because it is confusing to see that on *received* transactions.
Exampl
...
🤔 jonatack reviewed a pull request: "doc: Improve `dependencies.md`"
(https://github.com/bitcoin/bitcoin/pull/31895#pullrequestreview-2844215334)
ACK e62423d6f1514b022155edb5bc930cecc4236731
(https://github.com/bitcoin/bitcoin/pull/31895#pullrequestreview-2844215334)
ACK e62423d6f1514b022155edb5bc930cecc4236731
💬 hebasto commented on pull request "doc: Improve `dependencies.md`":
(https://github.com/bitcoin/bitcoin/pull/31895#discussion_r2091490497)
> Anything here seems fine, we don't link to https://sqlite.org/download.html or https://freetype.org/download.html.
But we do link to https://www.boost.org/users/download/ and https://github.com/libevent/libevent/releases.
I’ve ACKed the current branch and am content with it as is.
(https://github.com/bitcoin/bitcoin/pull/31895#discussion_r2091490497)
> Anything here seems fine, we don't link to https://sqlite.org/download.html or https://freetype.org/download.html.
But we do link to https://www.boost.org/users/download/ and https://github.com/libevent/libevent/releases.
I’ve ACKed the current branch and am content with it as is.
💬 hebasto commented on pull request "doc: Improve `dependencies.md`":
(https://github.com/bitcoin/bitcoin/pull/31895#discussion_r2091491960)
It was a nit. I am OK with the current branch.
(https://github.com/bitcoin/bitcoin/pull/31895#discussion_r2091491960)
It was a nit. I am OK with the current branch.
⚠️ fatmirsul1234 opened an issue: "f"
(https://github.com/bitcoin/bitcoin/issues/32518)
### Motivation
f
### Possible solution
_No response_
### Useful Skills
* Compiling Bitcoin Core from source
* Running the C++ unit tests and the Python functional tests
* ...
### Guidance for new contributors
Want to work on this issue?
For guidance on contributing, please read [CONTRIBUTING.md](https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md) before opening your pull request.
(https://github.com/bitcoin/bitcoin/issues/32518)
### Motivation
f
### Possible solution
_No response_
### Useful Skills
* Compiling Bitcoin Core from source
* Running the C++ unit tests and the Python functional tests
* ...
### Guidance for new contributors
Want to work on this issue?
For guidance on contributing, please read [CONTRIBUTING.md](https://github.com/bitcoin/bitcoin/blob/master/CONTRIBUTING.md) before opening your pull request.
✅ fanquake closed an issue: "f"
(https://github.com/bitcoin/bitcoin/issues/32518)
(https://github.com/bitcoin/bitcoin/issues/32518)
🚀 fanquake merged a pull request: "doc: Improve `dependencies.md`"
(https://github.com/bitcoin/bitcoin/pull/31895)
(https://github.com/bitcoin/bitcoin/pull/31895)
💬 Sjors commented on pull request "rfc: only put replaced txs in extra pool":
(https://github.com/bitcoin/bitcoin/pull/32510#issuecomment-2884350379)
@szarka the extra pool is capped at 100kb regardless of the number of transactions, so any filtered content over 100kb would overflow it.
It's (unfortunately) unclear from your log example what these 87 transactions were. If they were all RBF replacements, then this PR wouldn't hurt that. If they're all inscriptions, then it would.
> Seems like including things that are rejected for policy reasons is useful.
In principle yes. The problem is that when we decide a transaction is invalid f
...
(https://github.com/bitcoin/bitcoin/pull/32510#issuecomment-2884350379)
@szarka the extra pool is capped at 100kb regardless of the number of transactions, so any filtered content over 100kb would overflow it.
It's (unfortunately) unclear from your log example what these 87 transactions were. If they were all RBF replacements, then this PR wouldn't hurt that. If they're all inscriptions, then it would.
> Seems like including things that are rejected for policy reasons is useful.
In principle yes. The problem is that when we decide a transaction is invalid f
...
💬 szarka commented on pull request "rfc: only put replaced txs in extra pool":
(https://github.com/bitcoin/bitcoin/pull/32510#issuecomment-2884363317)
> In any case I'd love to see @0xB10C's data. I'll keep this in draft.
I think maybe [this _Delving_ post](https://delvingbitcoin.org/t/stats-on-compact-block-reconstructions/1052) is what @glozow was referring to?
(https://github.com/bitcoin/bitcoin/pull/32510#issuecomment-2884363317)
> In any case I'd love to see @0xB10C's data. I'll keep this in draft.
I think maybe [this _Delving_ post](https://delvingbitcoin.org/t/stats-on-compact-block-reconstructions/1052) is what @glozow was referring to?
💬 maflcko commented on pull request "rpc: add "ischange: true" to decoded tx outputs in wallet gettransaction response":
(https://github.com/bitcoin/bitcoin/pull/32517#discussion_r2091545479)
nit: Not sure about making this optional out of convenience and then write the rule in the description string.
It would be better to just pass down a `std::optionl<RPCResult>` (or `std::vector<RPCResult>`) here, and then use `Cat()` to concatenate it?
(https://github.com/bitcoin/bitcoin/pull/32517#discussion_r2091545479)
nit: Not sure about making this optional out of convenience and then write the rule in the description string.
It would be better to just pass down a `std::optionl<RPCResult>` (or `std::vector<RPCResult>`) here, and then use `Cat()` to concatenate it?
💬 m3dwards commented on pull request "ci: remove 3rd party js from windows dll gha job":
(https://github.com/bitcoin/bitcoin/pull/32513#discussion_r2091547152)
I spose I'd ask the question the other way round, why not the exception? It would be an exceptional circumstance if MSBuild could not be found.
(https://github.com/bitcoin/bitcoin/pull/32513#discussion_r2091547152)
I spose I'd ask the question the other way round, why not the exception? It would be an exceptional circumstance if MSBuild could not be found.