💬 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
...
(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
(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.
(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...)"} :
```
(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
(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
(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
(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.
(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
...
(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
(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`?
(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
(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.
(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
(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.
(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.
💬 achow101 commented on pull request "test: extend the SOCKS5 Python proxy to actually connect to a destination":
(https://github.com/bitcoin/bitcoin/pull/29420#issuecomment-2414389340)
cc @tdb3
(https://github.com/bitcoin/bitcoin/pull/29420#issuecomment-2414389340)
cc @tdb3
💬 BitcoinMechanic commented on pull request "set `DEFAULT_PERMIT_BAREMULTISIG` to false":
(https://github.com/bitcoin/bitcoin/pull/28217#issuecomment-2414391299)
Relaying anything by default that demonstrably only gets used for malicious purposes (which in this case was *openly stated by the perpetrator*) remains perplexing and embarrassing.
(https://github.com/bitcoin/bitcoin/pull/28217#issuecomment-2414391299)
Relaying anything by default that demonstrably only gets used for malicious purposes (which in this case was *openly stated by the perpetrator*) remains perplexing and embarrassing.
💬 achow101 commented on pull request "tracing: explicitly cast block_connected duration to nanoseconds":
(https://github.com/bitcoin/bitcoin/pull/29877#issuecomment-2414392442)
cc @tdb3
(https://github.com/bitcoin/bitcoin/pull/29877#issuecomment-2414392442)
cc @tdb3
💬 achow101 commented on pull request "validation: do not wipe utxo cache for stats/scans/snapshots":
(https://github.com/bitcoin/bitcoin/pull/30610#issuecomment-2414394453)
cc @davidgumberg @andrewtoth @l0rinc
(https://github.com/bitcoin/bitcoin/pull/30610#issuecomment-2414394453)
cc @davidgumberg @andrewtoth @l0rinc
📝 fanquake locked a pull request: "add `-limitdummyscriptdatasize` option"
(https://github.com/bitcoin/bitcoin/pull/29520)
Same as #28408, but with one new options:
`-limitdummyscriptdatasize` which allows you to choose the maximum size of the relayed dummy script with the default value `MAX_BLOCK_WEIGHT` (maximum size of a block)
PR based on the work of @luke-jr and @LarryRuane with the help of @nsvrn and @ChrisMartl
(https://github.com/bitcoin/bitcoin/pull/29520)
Same as #28408, but with one new options:
`-limitdummyscriptdatasize` which allows you to choose the maximum size of the relayed dummy script with the default value `MAX_BLOCK_WEIGHT` (maximum size of a block)
PR based on the work of @luke-jr and @LarryRuane with the help of @nsvrn and @ChrisMartl