Bitcoin Core Github
44 subscribers
120K links
Download Telegram
👍 stickies-v approved a pull request: "optimization: Preallocate addresses in GetAddr based on nNodes"
(https://github.com/bitcoin/bitcoin/pull/29608#pullrequestreview-2369709974)
ACK 66082ca3488e7ad78149e05631dccd09be03c961

I don't think this is a particularly impactful change but it's very small and straightforward to review.
achow101 closed a pull request: "add `-limitdummyscriptdatasize` option"
(https://github.com/bitcoin/bitcoin/pull/29520)
💬 achow101 commented on pull request "add `-limitdummyscriptdatasize` option":
(https://github.com/bitcoin/bitcoin/pull/29520#issuecomment-2414258657)
Closing this as it has not had any activity in a while, and feedback has not been addressed.
l0rinc closed a pull request: "CI: Add label to scripted-diffs"
(https://github.com/bitcoin/bitcoin/pull/31089)
💬 l0rinc commented on pull request "CI: Add label to scripted-diffs":
(https://github.com/bitcoin/bitcoin/pull/31089#issuecomment-2414260328)
> It could still be "hacked" by adding two scripted diffs, one that "fails" and one that passes and adds the label.

Valid concern, thanks for the comments, I'll close this in that case.
💬 jonatack commented on pull request "doc: add guidance for RPC to developer notes":
(https://github.com/bitcoin/bitcoin/pull/30142#discussion_r1801413165)
> Implement an associated `-deprecatedrpc=` option to retain previous RPC behavior when modifying the structure of RPC interface JSON in a backwards-incompatible manner.

Suggest to lead with what would require this, use simpler language, and mention the period:

"If an RPC will be changed in a backward-incompatible way, add an associated `-deprecatedrpc=` option to retain previous RPC behavior during the deprecation period."

> This includes, but is not limited to, ...

(Is this long se
...
💬 achow101 commented on pull request "coins: Add move operations to Coin and CCoinsCacheEntry":
(https://github.com/bitcoin/bitcoin/pull/30643#issuecomment-2414281260)
cc @davidgumberg @andrewtoth
💬 glozow commented on pull request "doc: add dustThreshold explain of P2SH & P2TR":
(https://github.com/bitcoin/bitcoin/pull/30023#discussion_r1801416152)
This specifically does not match the code.
💬 jonatack commented on pull request "doc: add guidance for RPC to developer notes":
(https://github.com/bitcoin/bitcoin/pull/30142#discussion_r1801427354)
Yes, we've also traditionally mentioned these in all-caps in the RPC helps, i.e.

```
RPCResult{RPCResult::Type::STR, "warnings", "(DEPRECATED) network and blockchain warnings, if any...)"} :
```
💬 achow101 commented on pull request "test: Shut down framework cleanly on RPC connection failure":
(https://github.com/bitcoin/bitcoin/pull/30660#issuecomment-2414327186)
cc @tdb3
💬 manjiechen commented on pull request "doc: add dustThreshold explain of P2SH & P2TR":
(https://github.com/bitcoin/bitcoin/pull/30023#discussion_r1801440711)
> This specifically does not match the code.

Here we want to explain the dust calculation logic of existing common scripts

The code below simplifies the logic of dust level and says in the comment that the dust limit will not be lowered for segwit v1

I think we need to explain the situation clearly, although we have some considerations in engineering to choose a general solution
💬 jasonandjay commented on pull request "doc: add dustThreshold explain of P2SH & P2TR":
(https://github.com/bitcoin/bitcoin/pull/30023#discussion_r1801442887)
> This specifically does not match the code.
Here we want to explain the dust calculation logic of existing common scripts

The code below simplifies the logic of dust level and says in the comment that the dust limit will not be lowered for segwitv1

I think we need to explain the situation clearly, although we have some considerations in engineering to choose a general solution
🤔 glozow reviewed a pull request: "include verbose "reject-details" field in testmempoolaccept response"
(https://github.com/bitcoin/bitcoin/pull/28121#pullrequestreview-2369763914)
Apologies about the late review! Sorry to bikeshed the interface a bit more, the rest looks good.
💬 glozow commented on pull request "include verbose "reject-details" field in testmempoolaccept response":
(https://github.com/bitcoin/bitcoin/pull/28121#discussion_r1801425455)
A simpler way to do this might just be `ToString()`.

It ensures the field exists as long as reject-reason exists. That'd be a simpler interface to understand, as the current one depends on very specific internal stuff - I can't really tell you off the top of my head where we have/don't add a debug string.

It does mean the (very short) first part of the error string is repeated in both places, but perhaps that is convenient for users that want the more verbose one.

Also nit, I heard unde
...
💬 glozow commented on pull request "include verbose "reject-details" field in testmempoolaccept response":
(https://github.com/bitcoin/bitcoin/pull/28121#discussion_r1801440138)
Can be ignored imo, it'd be deleted once copied to release notes draft anyway
💬 glozow commented on pull request "include verbose "reject-details" field in testmempoolaccept response":
(https://github.com/bitcoin/bitcoin/pull/28121#discussion_r1801438057)
any reason not to use `assert_equal`?
👍 stickies-v approved a pull request: "optimization: reserve memory allocation for transaction inputs/outputs"
(https://github.com/bitcoin/bitcoin/pull/30093#pullrequestreview-2369807087)
ACK ec585f11c38bbe277a596dcea3c813e7c6626050
💬 GregTonoski commented on pull request "add `-limitdummyscriptdatasize` option":
(https://github.com/bitcoin/bitcoin/pull/29520#issuecomment-2414355286)
> Closing this as it has not had any activity in a while, and feedback has not been addressed.

Disgraceful.
💬 achow101 commented on pull request "Tr partial descriptors":
(https://github.com/bitcoin/bitcoin/pull/30243#issuecomment-2414359345)
Needs a bip
💬 pinheadmz commented on pull request "add `-limitdummyscriptdatasize` option":
(https://github.com/bitcoin/bitcoin/pull/29520#issuecomment-2414362117)
@GregTonoski Your comment is off-topic and inflammatory, resulting in a 24 hour ban, please be respectful.