Bitcoin Core Github
44 subscribers
120K links
Download Telegram
šŸ’¬ tdb3 commented on pull request "Support JSON-RPC 2.0 when requested by client":
(https://github.com/bitcoin/bitcoin/pull/27101#issuecomment-1913628864)
ACK. Great work. I support the near-term goal of adding more correct handling of JSON-RPC 2.0 and the longer-term goal of deprecating 1.x, culminating in the eventual removal of 1.x support (and cleanup from refactoring out support of 1.x).

Noting findings/checks for archival purposes:
Cloned, checked out the PR branch (98031401c744ea3c2b62927fabf27c22b639c3cf), built, ran all functional tests (all passed), then executed bitcoind against a private signet. Issued RPC requests with curl. I
...
šŸ¤” tdb3 reviewed a pull request: "Support JSON-RPC 2.0 when requested by client"
(https://github.com/bitcoin/bitcoin/pull/27101#pullrequestreview-1847563884)
ACK. Great work. I support the near-term goal of adding more correct handling of JSON-RPC 2.0 and the longer-term goal of deprecating 1.x, culminating in the eventual removal of 1.x support (and cleanup from refactoring out support of 1.x).

Noting for archival purposes:
Cloned, checked out the PR branch (98031401c744ea3c2b62927fabf27c22b639c3cf), built, ran all functional tests (all passed), then executed bitcoind against a private signet. Issued RPC requests with curl (getblockchaininfo
...
šŸ’¬ tdb3 commented on pull request "Support JSON-RPC 2.0 when requested by client":
(https://github.com/bitcoin/bitcoin/pull/27101#discussion_r1468881262)
nit:
Minor non-compliance to the JSON RPC 2.0 spec (`rpc call with an empty Array` in https://www.jsonrpc.org/specification#examples). Providing an empty array results in an empty array provided in the response, but should result in a single error response (e.g. `"jsonrpc": "2.0", "error": {"code": -32600, "message": "Invalid Request"}, "id": null}`).

A check for `valRequest.size() == 0` before entering the for loop in 236 could enable error response (rather than empty response array). P
...
šŸ’¬ tdb3 commented on pull request "Support JSON-RPC 2.0 when requested by client":
(https://github.com/bitcoin/bitcoin/pull/27101#discussion_r1468881540)
Issuing an example command from https://developer.bitcoin.org/reference/rpc/getblockchaininfo.html resulted in error response `{"result":null,"error":{"code":-32600,"message":"JSON-RPC version not supported"}}` due to the example's use of `"jsonrpc":"1.0"`. It looks like JSON RPC spec for 1.0 had no version field in the spec, 1.1 (WD/ALT) had the "version" field to specify version, and 2.0 spec has "jsonrpc" (must be set to "2.0").

If a client uses a "jsonrpc" field other than "2.0" it wil
...
šŸ’¬ RicYashiroLee commented on pull request "set `DEFAULT_PERMIT_BAREMULTISIG` to false":
(https://github.com/bitcoin/bitcoin/pull/28217#issuecomment-1913671308)
> > Why was my last comment to Peter Todd's comment, DELETED? Where can I find the reasoning for such deletion?
>
> Your comment was deleted because it was off topic, consisted of insults, and contributed nothing to the discussion. Continue to make such comments and you will be blocked from this repo.

@achow101 can I ask that my original comment, which originated your moderation, be shown here also in this "pushed aside" section of the PR? Just so anyone understands better your reasoning,
...
šŸ’¬ achow101 commented on pull request "set `DEFAULT_PERMIT_BAREMULTISIG` to false":
(https://github.com/bitcoin/bitcoin/pull/28217#issuecomment-1913679571)
> > > Why was my last comment to Peter Todd's comment, DELETED? Where can I find the reasoning for such deletion?
> >
> >
> > Your comment was deleted because it was off topic, consisted of insults, and contributed nothing to the discussion. Continue to make such comments and you will be blocked from this repo.
>
> @achow101 can I ask that my original comment, which originated your moderation, be shown here also in this "pushed aside" section of the PR? Just so anyone understands better
...
šŸ’¬ RicYashiroLee commented on pull request "set `DEFAULT_PERMIT_BAREMULTISIG` to false":
(https://github.com/bitcoin/bitcoin/pull/28217#issuecomment-1913686703)
> > > > Why was my last comment to Peter Todd's comment, DELETED? Where can I find the reasoning for such deletion?
> > >
> > >
> > > Your comment was deleted because it was off topic, consisted of insults, and contributed nothing to the discussion. Continue to make such comments and you will be blocked from this repo.
> >
> >
> > @achow101 can I ask that my original comment, which originated your moderation, be shown here also in this "pushed aside" section of the PR? Just so anyone u
...
šŸ’¬ achow101 commented on pull request "set `DEFAULT_PERMIT_BAREMULTISIG` to false":
(https://github.com/bitcoin/bitcoin/pull/28217#issuecomment-1913691938)
> You keep warning me that I can get blocked, you can imagine that creates a situation that is unpleasant and threatening to someone on a first warning immediately, should not sound like a last one, right?

A warning should include all information about what will happen if the violation occurs again. If you continue to make off topic and abusive posts, you will be temporarily blocked. If you continue to do so once that is up, you will be permanently block. So indeed, this warning is both your
...
šŸ’¬ owenstrevor commented on pull request "Make provably unsignable standard P2PK and P2MS outpoints unspendable.":
(https://github.com/bitcoin/bitcoin/pull/28400#issuecomment-1913693984)
So this would make all current and future STAMPS unspendable? Or just future ones? How would this affect other potential legitimate use cases?

What is the projected growth of node requirements if this situation were to get worse and how does that track with Moore's Law of the increase in computer performance? Or is there a better way to think about this?
āš ļø techy2 opened an issue: "loadtxoutset fails to recognize assumeutxo blockhash"
(https://github.com/bitcoin/bitcoin/issues/29340)
### Is there an existing issue for this?

- [X] I have searched the existing issues

### Current behaviour

Unable to load UTXO snapshot, assumeutxo block hash in snapshot metadata not
recognized (00000000000000000000d4041fe771edd9dcb945bb85dfa739af1d2724a464e8)
(code -32603)

### Expected behaviour

expect txoutset to load properly

after researching issues, can't tell if this is new or has been recently fixed, so reporting as new

### Steps to reproduce

on 64 bit ubuntu bionic bitcoin-q
...
šŸ’¬ dzyphr commented on pull request "Make provably unsignable standard P2PK and P2MS outpoints unspendable.":
(https://github.com/bitcoin/bitcoin/pull/28400#issuecomment-1913753758)
> So this would make all current and future STAMPS unspendable? Or just future ones? How would this affect other potential legitimate use cases?
>
> What is the projected growth of node requirements if this situation were to get worse and how does that track with Moore's Law of the increase in computer performance? Or is there a better way to think about this?
>
> It's worth noting, making these unspendable may not stop their proliferation. For example, solutions that allow you to trade pr
...
āœ… achow101 closed an issue: "loadtxoutset fails to recognize assumeutxo blockhash"
(https://github.com/bitcoin/bitcoin/issues/29340)
šŸ’¬ achow101 commented on issue "loadtxoutset fails to recognize assumeutxo blockhash":
(https://github.com/bitcoin/bitcoin/issues/29340#issuecomment-1913754353)
Loaded utxo sets need to match hardcoded utxo set and block hashes. These are currently not set for mainnet and so `loadtxoutset` is not expected to work on mainnet.
šŸ’¬ dooglus commented on issue "New crash in v26.0":
(https://github.com/bitcoin-core/gui/issues/785#issuecomment-1913757550)
> It seems you are running a prune node. Please with the logs, provide your bitcoin.conf values (hiding the sensitive information).

This is what's in the pruned node's bitcoin.conf:

```
connect=localhost
prune=555
addresstype=bech32
keypool=10
upnp=0
spendzeroconfchange=1
avoidpartialspends=1
server=1
daemon=0
checkblocks=1
checklevel=0
logips=1
rpcport=8550
port=8551
wallet=a
wallet=b
wallet=c
wallet=d
deprecatedrpc=accounts
rpcuser=user
rpcpassword=pass
rpcauth=u1:p
...
šŸ¤” murchandamus reviewed a pull request: "wallet: Add CoinGrinder coin selection algorithm"
(https://github.com/bitcoin/bitcoin/pull/27877#pullrequestreview-1847632560)
Addressed most of @sipa’s feedback except for two things I’m still mulling over
šŸ’¬ murchandamus commented on pull request "wallet: Add CoinGrinder coin selection algorithm":
(https://github.com/bitcoin/bitcoin/pull/27877#discussion_r1468939798)
Good catch, fixed.
šŸ’¬ murchandamus commented on pull request "wallet: Add CoinGrinder coin selection algorithm":
(https://github.com/bitcoin/bitcoin/pull/27877#discussion_r1468939922)
Sure, makes sense.
šŸ’¬ murchandamus commented on pull request "wallet: Add CoinGrinder coin selection algorithm":
(https://github.com/bitcoin/bitcoin/pull/27877#discussion_r1468941852)
Thanks, fixed up the sentence.
šŸ’¬ murchandamus commented on pull request "wallet: Add CoinGrinder coin selection algorithm":
(https://github.com/bitcoin/bitcoin/pull/27877#discussion_r1468941377)
Added descriptions

```cpp
// The current selection and the best input set found so far, stored as the utxo_pool indices of the UTXOs forming them
std::vector<size_t> curr_selection;
std::vector<size_t> best_selection;

// The currently selected effective amount, and the effective amount of the best selection so far
CAmount curr_amount = 0;
CAmount best_selection_amount = MAX_MONEY;

// The weight of the currently selected input set, and the weight of the best selection
int curr_weig
...
šŸ’¬ murchandamus commented on pull request "wallet: Add CoinGrinder coin selection algorithm":
(https://github.com/bitcoin/bitcoin/pull/27877#discussion_r1468960861)
That would be good, but that would take more time than I have right now. Will leave this open for the moment.