💬 criadoperez commented on pull request "typos on src files":
(https://github.com/bitcoin/bitcoin/pull/28022#issuecomment-1618234145)
I'll do that then. Thanks!
(https://github.com/bitcoin/bitcoin/pull/28022#issuecomment-1618234145)
I'll do that then. Thanks!
💬 willcl-ark commented on pull request "init: deduplicate added connections":
(https://github.com/bitcoin/bitcoin/pull/27804#issuecomment-1618247262)
Thanks Luke, I mostly agree (that this is a bit hacky). Implemented here currently is the "simplest" (could also read "dumbest") fix to the issue, whilst trying to avoid any using more mutexes and possibly introducing new thread safety issues etc.
As I see it the root cause is that we use two threads to make outbound connections simultaneously with no way of deduplicating except _after_ the connection is successful and the node is pushed onto `m_nodes`. This allows the race on node connection
...
(https://github.com/bitcoin/bitcoin/pull/27804#issuecomment-1618247262)
Thanks Luke, I mostly agree (that this is a bit hacky). Implemented here currently is the "simplest" (could also read "dumbest") fix to the issue, whilst trying to avoid any using more mutexes and possibly introducing new thread safety issues etc.
As I see it the root cause is that we use two threads to make outbound connections simultaneously with no way of deduplicating except _after_ the connection is successful and the node is pushed onto `m_nodes`. This allows the race on node connection
...
💬 ItIsOHM commented on issue "rpc: doc: RPCHelpMan missing request parameters for getblocktemplate":
(https://github.com/bitcoin/bitcoin/issues/27998#issuecomment-1618251162)
> Absolutely, just open the PR whenever you feel ready, or ask away here if you have any problems you can't figure out.
Hey @stickies-v! So as a starter, I don't have bitcoin-cli working, so to do that I'm building the core with Visual Studio by following this guide : https://github.com/bitcoin/bitcoin/blob/master/build_msvc/README.md, is that alright or do I need to follow some other step to use `bitcoin-cli`?
(https://github.com/bitcoin/bitcoin/issues/27998#issuecomment-1618251162)
> Absolutely, just open the PR whenever you feel ready, or ask away here if you have any problems you can't figure out.
Hey @stickies-v! So as a starter, I don't have bitcoin-cli working, so to do that I'm building the core with Visual Studio by following this guide : https://github.com/bitcoin/bitcoin/blob/master/build_msvc/README.md, is that alright or do I need to follow some other step to use `bitcoin-cli`?
📝 MarcoFalke opened a pull request: "ci: Remove deprecated container.greedy"
(https://github.com/bitcoin/bitcoin/pull/28024)
The option is to be phased out, so remove it to avoid relying on it. Update container.cpu and timeouts where needed.
(https://github.com/bitcoin/bitcoin/pull/28024)
The option is to be phased out, so remove it to avoid relying on it. Update container.cpu and timeouts where needed.
💬 Ayush170-Future commented on pull request "init: adding check for : for -torcontrol flag":
(https://github.com/bitcoin/bitcoin/pull/28018#issuecomment-1618315720)
Tested this locally.
1. Any string that is not of the type `<host>:<port>` is rejected.
Example: `./src/bitcoind -torcontrol=1`.
2. Also works for Domain names.
Example: `./src/bitcoind -torcontrol=localhost:9051` is accepted.
3. But multiple instances of `:` are also accepted.
Example: `./src/bitcoind -torcontrol=localhost:9051:23` is also accepted.
This should also be avoided, right?
> Also another thing I wanted to clarify is that will `-torcontrol` always be in the forma
...
(https://github.com/bitcoin/bitcoin/pull/28018#issuecomment-1618315720)
Tested this locally.
1. Any string that is not of the type `<host>:<port>` is rejected.
Example: `./src/bitcoind -torcontrol=1`.
2. Also works for Domain names.
Example: `./src/bitcoind -torcontrol=localhost:9051` is accepted.
3. But multiple instances of `:` are also accepted.
Example: `./src/bitcoind -torcontrol=localhost:9051:23` is also accepted.
This should also be avoided, right?
> Also another thing I wanted to clarify is that will `-torcontrol` always be in the forma
...
💬 hebasto commented on pull request "ci: Remove deprecated container.greedy":
(https://github.com/bitcoin/bitcoin/pull/28024#issuecomment-1618317711)
> The option is to be phased out,
It is still mentioned in https://cirrus-ci.org/guide/linux/#linux-containers.
(https://github.com/bitcoin/bitcoin/pull/28024#issuecomment-1618317711)
> The option is to be phased out,
It is still mentioned in https://cirrus-ci.org/guide/linux/#linux-containers.
💬 ItIsOHM commented on issue "rpc: doc: RPCHelpMan missing request parameters for getblocktemplate":
(https://github.com/bitcoin/bitcoin/issues/27998#issuecomment-1618323403)
Update: I downloaded and installed `Bitcoin Core`, which then helped me to run `bitcoind` and `bitcoin-cli`. I'll try to make the changes to a new branch :D
(https://github.com/bitcoin/bitcoin/issues/27998#issuecomment-1618323403)
Update: I downloaded and installed `Bitcoin Core`, which then helped me to run `bitcoind` and `bitcoin-cli`. I'll try to make the changes to a new branch :D
💬 stickies-v commented on issue "rpc: doc: RPCHelpMan missing request parameters for getblocktemplate":
(https://github.com/bitcoin/bitcoin/issues/27998#issuecomment-1618332163)
MSVC is one option, WSL another. See also https://github.com/bitcoin/bitcoin/blob/master/doc/build-windows.md.
Build questions are a bit out of scope for this issue though, there's a fair amount of documentation online. Just downloading the binaries isn't enough to work on this, you need to compile it yourself to verify the changes are correct.
(https://github.com/bitcoin/bitcoin/issues/27998#issuecomment-1618332163)
MSVC is one option, WSL another. See also https://github.com/bitcoin/bitcoin/blob/master/doc/build-windows.md.
Build questions are a bit out of scope for this issue though, there's a fair amount of documentation online. Just downloading the binaries isn't enough to work on this, you need to compile it yourself to verify the changes are correct.
💬 Ayush170-Future commented on pull request "init: adding check for : for -torcontrol flag":
(https://github.com/bitcoin/bitcoin/pull/28018#discussion_r1250929352)
I believe here. You might additionally check the count of `:` characters in `socket_addr` and return an error if they are more than one.
(https://github.com/bitcoin/bitcoin/pull/28018#discussion_r1250929352)
I believe here. You might additionally check the count of `:` characters in `socket_addr` and return an error if they are more than one.
🤔 Sjors reviewed a pull request: "[WIP] add a stratum v2 template provider"
(https://github.com/bitcoin/bitcoin/pull/27854#pullrequestreview-1511170547)
The PR description could use instructions on how to test this. Specifically I'd like to try it on mainnet against my vintage S9 with BraiinsOS+ (not sure which pools already support job negotiation/ announcement). This can of course point to existing documentation. [Config D](https://stratumprotocol.org/getting-started/#config-d-sv1-firmware--translation-proxy-jn-job-negotiator--sv2-pool) is close but it assumes regtest and running your own pool.
It will also be easier to test and review this
...
(https://github.com/bitcoin/bitcoin/pull/27854#pullrequestreview-1511170547)
The PR description could use instructions on how to test this. Specifically I'd like to try it on mainnet against my vintage S9 with BraiinsOS+ (not sure which pools already support job negotiation/ announcement). This can of course point to existing documentation. [Config D](https://stratumprotocol.org/getting-started/#config-d-sv1-firmware--translation-proxy-jn-job-negotiator--sv2-pool) is close but it assumes regtest and running your own pool.
It will also be easier to test and review this
...
💬 Sjors commented on pull request "[WIP] add a stratum v2 template provider":
(https://github.com/bitcoin/bitcoin/pull/27854#discussion_r1250873859)
This should also mention the testnet, regtest and signet ports.
(https://github.com/bitcoin/bitcoin/pull/27854#discussion_r1250873859)
This should also mention the testnet, regtest and signet ports.
💬 ItIsOHM commented on issue "rpc: doc: RPCHelpMan missing request parameters for getblocktemplate":
(https://github.com/bitcoin/bitcoin/issues/27998#issuecomment-1618350216)
> MSVC is one option, WSL another. See also https://github.com/bitcoin/bitcoin/blob/master/doc/build-windows.md.
>
> Build questions are a bit out of scope for this issue though, there's a fair amount of documentation online. Just downloading the binaries isn't enough to work on this, you need to compile it yourself to verify the changes are correct.
Before using `Bitcoin Core` I actually used `build_msvc` which gave me a bunch folders with `*.vcxproj` files, however I was unsure where to
...
(https://github.com/bitcoin/bitcoin/issues/27998#issuecomment-1618350216)
> MSVC is one option, WSL another. See also https://github.com/bitcoin/bitcoin/blob/master/doc/build-windows.md.
>
> Build questions are a bit out of scope for this issue though, there's a fair amount of documentation online. Just downloading the binaries isn't enough to work on this, you need to compile it yourself to verify the changes are correct.
Before using `Bitcoin Core` I actually used `build_msvc` which gave me a bunch folders with `*.vcxproj` files, however I was unsure where to
...
🤔 furszy reviewed a pull request: "wallet: don't duplicate change output if already exist"
(https://github.com/bitcoin/bitcoin/pull/27601#pullrequestreview-1511239594)
> How would it work if coinselection produced result without change output?
There is a test exercising the behavior inside a8ac2ad8 and also an explanation there.
But essentially, the process must adhere to the outputs specified by the user. If the change destination option in 'fundrawtransaction' matches one of the output scripts, the wallet is instructed to 'increase the specified output only if there is any change' and must not discard any output predefined by the user under any circums
...
(https://github.com/bitcoin/bitcoin/pull/27601#pullrequestreview-1511239594)
> How would it work if coinselection produced result without change output?
There is a test exercising the behavior inside a8ac2ad8 and also an explanation there.
But essentially, the process must adhere to the outputs specified by the user. If the change destination option in 'fundrawtransaction' matches one of the output scripts, the wallet is instructed to 'increase the specified output only if there is any change' and must not discard any output predefined by the user under any circums
...
💬 furszy commented on pull request "wallet: don't duplicate change output if already exist":
(https://github.com/bitcoin/bitcoin/pull/27601#discussion_r1250923477)
> Isn't the fee for all outputs accounted for in not_input_fees? It seems to me this will always overpay and then will be corrected when we detect surplus fee.
>
> Maybe a clearer way to handle this would be to pass chnage_fee=0 to GetChange if existent_change_out_index=true. I'd prefer to minimise number of cases when we need to correct fees at the later stage of transaction building.
Good point.
Could go further and set the coin control `m_change_fee` and `min_viable_change` to 0 so coin
...
(https://github.com/bitcoin/bitcoin/pull/27601#discussion_r1250923477)
> Isn't the fee for all outputs accounted for in not_input_fees? It seems to me this will always overpay and then will be corrected when we detect surplus fee.
>
> Maybe a clearer way to handle this would be to pass chnage_fee=0 to GetChange if existent_change_out_index=true. I'd prefer to minimise number of cases when we need to correct fees at the later stage of transaction building.
Good point.
Could go further and set the coin control `m_change_fee` and `min_viable_change` to 0 so coin
...
👍 fanquake approved a pull request: "ci: Remove deprecated container.greedy"
(https://github.com/bitcoin/bitcoin/pull/28024#pullrequestreview-1511281401)
ACK fa248a4c616be31d231c671e8feb0dbb46ac54cd
(https://github.com/bitcoin/bitcoin/pull/28024#pullrequestreview-1511281401)
ACK fa248a4c616be31d231c671e8feb0dbb46ac54cd
💬 hebasto commented on pull request "Exit and show error if unrecognized command line args are present":
(https://github.com/bitcoin-core/gui/pull/742#issuecomment-1618418896)
Is it safer and simpler to just document that a payment URi must follow all command line options? In `bitcoin-qt -help`?
(https://github.com/bitcoin-core/gui/pull/742#issuecomment-1618418896)
Is it safer and simpler to just document that a payment URi must follow all command line options? In `bitcoin-qt -help`?
💬 fanquake commented on pull request "guix: use GCC 12.2.0 to build releases":
(https://github.com/bitcoin/bitcoin/pull/27897#issuecomment-1618420576)
@theuni I will address your comments soon. Currently working on some additional changes so we can push the time-machine commit further along, and potentially take advantage of the now merged: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=64100. I've opened one additional issue upstream: https://lists.gnu.org/archive/html/bug-guix/2023-07/msg00009.html.
(https://github.com/bitcoin/bitcoin/pull/27897#issuecomment-1618420576)
@theuni I will address your comments soon. Currently working on some additional changes so we can push the time-machine commit further along, and potentially take advantage of the now merged: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=64100. I've opened one additional issue upstream: https://lists.gnu.org/archive/html/bug-guix/2023-07/msg00009.html.
💬 fanquake commented on issue "Intermittent failures in interface_usdt_mempool.py":
(https://github.com/bitcoin/bitcoin/issues/27380#issuecomment-1618430203)
https://cirrus-ci.com/task/6504094303518720?logs=ci#L3264
```bash
test 2023-07-03T14:16:38.121000Z TestFramework (INFO): Polling buffer...
node0 2023-07-03T14:16:38.121067Z (mocktime: 2023-07-17T14:16:35Z) [http] [httpserver.cpp:255] [http_request_cb] [http] Received a POST request for / from 127.0.0.1:40312
node0 2023-07-03T14:16:38.121207Z (mocktime: 2023-07-17T14:16:35Z) [httpworker.2] [rpc/request.cpp:181] [parse] [rpc] ThreadRPCServer method=getrawmempool user=__cookie__
test
...
(https://github.com/bitcoin/bitcoin/issues/27380#issuecomment-1618430203)
https://cirrus-ci.com/task/6504094303518720?logs=ci#L3264
```bash
test 2023-07-03T14:16:38.121000Z TestFramework (INFO): Polling buffer...
node0 2023-07-03T14:16:38.121067Z (mocktime: 2023-07-17T14:16:35Z) [http] [httpserver.cpp:255] [http_request_cb] [http] Received a POST request for / from 127.0.0.1:40312
node0 2023-07-03T14:16:38.121207Z (mocktime: 2023-07-17T14:16:35Z) [httpworker.2] [rpc/request.cpp:181] [parse] [rpc] ThreadRPCServer method=getrawmempool user=__cookie__
test
...
🤔 furszy reviewed a pull request: "Debug Console implementation of generate method"
(https://github.com/bitcoin-core/gui/pull/692#pullrequestreview-1491894680)
Left a quick review, not really blocking. Mostly performance and code improvements.
Will finish testing it later today and ACK it if all goes well.
(https://github.com/bitcoin-core/gui/pull/692#pullrequestreview-1491894680)
Left a quick review, not really blocking. Mostly performance and code improvements.
Will finish testing it later today and ACK it if all goes well.
💬 furszy commented on pull request "Debug Console implementation of generate method":
(https://github.com/bitcoin-core/gui/pull/692#discussion_r1248843611)
nit: format:
```suggestion
Q_EMIT reply(RPCConsole::CMD_ERROR, QString("Error: could not generate blocks"));
```
(https://github.com/bitcoin-core/gui/pull/692#discussion_r1248843611)
nit: format:
```suggestion
Q_EMIT reply(RPCConsole::CMD_ERROR, QString("Error: could not generate blocks"));
```