💬 remyers commented on pull request "wallet: fix coin selection tracing to return -1 when no change pos":
(https://github.com/bitcoin/bitcoin/pull/29272#discussion_r1458048472)
I had my casting on the wrong value - we need to cast the unsigned optional value. Fixed in [e701982](https://github.com/bitcoin/bitcoin/pull/29272/commits/e701982c741edd2182690ddf4628e4c079a5185e).
(https://github.com/bitcoin/bitcoin/pull/29272#discussion_r1458048472)
I had my casting on the wrong value - we need to cast the unsigned optional value. Fixed in [e701982](https://github.com/bitcoin/bitcoin/pull/29272/commits/e701982c741edd2182690ddf4628e4c079a5185e).
💬 wizkid057 commented on issue "Witness scripts being abused to bypass datacarriersize limit (CVE-2023-50428)":
(https://github.com/bitcoin/bitcoin/issues/29187#issuecomment-1899379741)
> > Again, let's be clear: Cybersecurity 101 stuff here. The ability to inject arbitrary data into any system in a way that isn't explicitly documented as permitted for an expected purpose of the system is always a vulnerability and always something that should be addressed.
>
> You can use nlocktime and amounts in outputs to decode it to some text. Will that need a CVE ID?
You're clearly using a bit of hyperbole here, again in yet another attempt at a "gotcha" moment without addressing _a
...
(https://github.com/bitcoin/bitcoin/issues/29187#issuecomment-1899379741)
> > Again, let's be clear: Cybersecurity 101 stuff here. The ability to inject arbitrary data into any system in a way that isn't explicitly documented as permitted for an expected purpose of the system is always a vulnerability and always something that should be addressed.
>
> You can use nlocktime and amounts in outputs to decode it to some text. Will that need a CVE ID?
You're clearly using a bit of hyperbole here, again in yet another attempt at a "gotcha" moment without addressing _a
...
💬 1440000bytes commented on pull request "set `DEFAULT_PERMIT_BAREMULTISIG` to false":
(https://github.com/bitcoin/bitcoin/pull/28217#issuecomment-1899384781)
> tACK
You can add the commit that you tested after tACK: https://github.com/bitcoin/bitcoin/pull/28217/commits/8672721deb06e66854a085c9994e99c99160b8a1
Or more details what exactly was tested and reasons to agree with pull request.
(https://github.com/bitcoin/bitcoin/pull/28217#issuecomment-1899384781)
> tACK
You can add the commit that you tested after tACK: https://github.com/bitcoin/bitcoin/pull/28217/commits/8672721deb06e66854a085c9994e99c99160b8a1
Or more details what exactly was tested and reasons to agree with pull request.
💬 1440000bytes commented on issue "Witness scripts being abused to bypass datacarriersize limit (CVE-2023-50428)":
(https://github.com/bitcoin/bitcoin/issues/29187#issuecomment-1899396269)
> Storing data in nLockTime is not happening as it's not really even exploitable. 32-bits max, but far less in practice, since would need to be a value permitted to be included in a block... so maybe 27 bits or so max per transaction? Just use OP_RETURN. Cheaper and more efficient.
https://x.com/1440000bytes/status/1732580146203250731
https://bitcoin.stackexchange.com/questions/23792/can-someone-explain-nlocktime/
> Store data in amounts is convoluted since you'd need a significant amou
...
(https://github.com/bitcoin/bitcoin/issues/29187#issuecomment-1899396269)
> Storing data in nLockTime is not happening as it's not really even exploitable. 32-bits max, but far less in practice, since would need to be a value permitted to be included in a block... so maybe 27 bits or so max per transaction? Just use OP_RETURN. Cheaper and more efficient.
https://x.com/1440000bytes/status/1732580146203250731
https://bitcoin.stackexchange.com/questions/23792/can-someone-explain-nlocktime/
> Store data in amounts is convoluted since you'd need a significant amou
...
💬 wizkid057 commented on issue "Witness scripts being abused to bypass datacarriersize limit (CVE-2023-50428)":
(https://github.com/bitcoin/bitcoin/issues/29187#issuecomment-1899406273)
Things you've noted are addressed by the availability of OP_RETURN.
(https://github.com/bitcoin/bitcoin/issues/29187#issuecomment-1899406273)
Things you've noted are addressed by the availability of OP_RETURN.
💬 stickies-v commented on pull request "RPC: access RPC arguments by name":
(https://github.com/bitcoin/bitcoin/pull/29277#issuecomment-1899415885)
Force pushed to make CI happy:
- removed doxygen @params since it didn't pick up the overloads (and it's already documented in plain-text anyway)
- added a null return type to the testing `RPCHelpMan`
(https://github.com/bitcoin/bitcoin/pull/29277#issuecomment-1899415885)
Force pushed to make CI happy:
- removed doxygen @params since it didn't pick up the overloads (and it's already documented in plain-text anyway)
- added a null return type to the testing `RPCHelpMan`
📝 theStack opened a pull request: "test: p2p: check disconnect due to lack of desirable service flags"
(https://github.com/bitcoin/bitcoin/pull/29279)
This PR adds missing test coverage for disconnecting peers which don't offer the desirable service flags in their VERSION message:
https://github.com/bitcoin/bitcoin/blob/5f3a0574c45477288bc678b15f24940486084576/src/net_processing.cpp#L3384-L3389
This check is relevant for the connection types "outbound-full-relay", "block-relay-only" and "addr-fetch" (see `CNode::ExpectServicesFromConn(...)`). Feeler connections always disconnect, which is also tested here.
In lack of finding a proper file
...
(https://github.com/bitcoin/bitcoin/pull/29279)
This PR adds missing test coverage for disconnecting peers which don't offer the desirable service flags in their VERSION message:
https://github.com/bitcoin/bitcoin/blob/5f3a0574c45477288bc678b15f24940486084576/src/net_processing.cpp#L3384-L3389
This check is relevant for the connection types "outbound-full-relay", "block-relay-only" and "addr-fetch" (see `CNode::ExpectServicesFromConn(...)`). Feeler connections always disconnect, which is also tested here.
In lack of finding a proper file
...
📝 reardencode opened a pull request: "Implement OP_CHECKTEMPLATEVERIFY"
(https://github.com/bitcoin/bitcoin/pull/29280)
This pull request forks from #29198 and only includes the implementation of OP_CHECKTEMPLATEVERIFY, its tests (except the functional ones), and standardization of bare OP_CHECKTEMPLATEVERIFY locking scripts.
(https://github.com/bitcoin/bitcoin/pull/29280)
This pull request forks from #29198 and only includes the implementation of OP_CHECKTEMPLATEVERIFY, its tests (except the functional ones), and standardization of bare OP_CHECKTEMPLATEVERIFY locking scripts.
💬 ajtowns commented on pull request "Improve new LogDebug/Trace/Info/Warning/Error Macros":
(https://github.com/bitcoin/bitcoin/pull/29256#issuecomment-1899825975)
> Agree, but the idea here is not to discard log messages, the idea just is to attach meaningful metadata to log messages so they can be filtered by component.
Filtering is discarding.
If you just want to *find* log messages, that what's grep is for, and if you want to make it more fine-grained than "hey this is an important log message/warning/error", that's what `-logsourcelocations` is for.
If it's any help, I think it's better to think of the sections as not related to which section
...
(https://github.com/bitcoin/bitcoin/pull/29256#issuecomment-1899825975)
> Agree, but the idea here is not to discard log messages, the idea just is to attach meaningful metadata to log messages so they can be filtered by component.
Filtering is discarding.
If you just want to *find* log messages, that what's grep is for, and if you want to make it more fine-grained than "hey this is an important log message/warning/error", that's what `-logsourcelocations` is for.
If it's any help, I think it's better to think of the sections as not related to which section
...
💬 Apdlrcjafg19 commented on issue "Libbitcoinkernel Project Tracking":
(https://github.com/bitcoin/bitcoin/issues/27587#issuecomment-1899940946)
> #### Project Board: https://github.com/orgs/bitcoin/projects/3
> This is the tracking issue for the `libbitcoinkernel` project. The original tracking issue is found in [#24303](https://github.com/bitcoin/bitcoin/issues/24303). This issue contains much of the content written by Carl Dong in the original issue but is more regularly updated.
>
> The libbitcoinkernel project is a new attempt at extracting our consensus engine. The kernel part of the name highlights one of the key functional diffe
...
(https://github.com/bitcoin/bitcoin/issues/27587#issuecomment-1899940946)
> #### Project Board: https://github.com/orgs/bitcoin/projects/3
> This is the tracking issue for the `libbitcoinkernel` project. The original tracking issue is found in [#24303](https://github.com/bitcoin/bitcoin/issues/24303). This issue contains much of the content written by Carl Dong in the original issue but is more regularly updated.
>
> The libbitcoinkernel project is a new attempt at extracting our consensus engine. The kernel part of the name highlights one of the key functional diffe
...
💬 ktecho commented on issue "Witness scripts being abused to bypass datacarriersize limit (CVE-2023-50428)":
(https://github.com/bitcoin/bitcoin/issues/29187#issuecomment-1899954524)
> Which of these [opcodes](https://en.bitcoin.it/wiki/Script#Opcodes) is not working as implemented or [documented](https://github.com/ChrisCho-H/bitcoin/blob/0bf55345764d22c5653c0a374647aa5582a186a2/src/script/script.h)?
It's not a single opcode that is not working as implemented or documented. It's the whole system that is not behaving as expected.
If you have an API so users of your site can upload pictures for blog posts, and you found that somebody uploaded a 35 GB blueray-ripped movi
...
(https://github.com/bitcoin/bitcoin/issues/29187#issuecomment-1899954524)
> Which of these [opcodes](https://en.bitcoin.it/wiki/Script#Opcodes) is not working as implemented or [documented](https://github.com/ChrisCho-H/bitcoin/blob/0bf55345764d22c5653c0a374647aa5582a186a2/src/script/script.h)?
It's not a single opcode that is not working as implemented or documented. It's the whole system that is not behaving as expected.
If you have an API so users of your site can upload pictures for blog posts, and you found that somebody uploaded a 35 GB blueray-ripped movi
...
💬 1ma commented on pull request "set `DEFAULT_PERMIT_BAREMULTISIG` to false":
(https://github.com/bitcoin/bitcoin/pull/28217#issuecomment-1899975514)
> > tACK
>
> You can add the commit that you tested after tACK: [8672721](https://github.com/bitcoin/bitcoin/commit/8672721deb06e66854a085c9994e99c99160b8a1)
>
> Or more details what exactly was tested and reasons to agree with pull request.
I already gave my reasons back in August, top of the thread. This isn't Twitter, if you want to participate here make an effort to read everything through an through.
As for what was tested, obviously the full PR. I built @Retropex's branch on my
...
(https://github.com/bitcoin/bitcoin/pull/28217#issuecomment-1899975514)
> > tACK
>
> You can add the commit that you tested after tACK: [8672721](https://github.com/bitcoin/bitcoin/commit/8672721deb06e66854a085c9994e99c99160b8a1)
>
> Or more details what exactly was tested and reasons to agree with pull request.
I already gave my reasons back in August, top of the thread. This isn't Twitter, if you want to participate here make an effort to read everything through an through.
As for what was tested, obviously the full PR. I built @Retropex's branch on my
...
💬 vasild commented on pull request "wallet: avoid returning a reference to vMasterKey after releasing the mutex that guards it":
(https://github.com/bitcoin/bitcoin/pull/28774#issuecomment-1899991600)
> Would suggest replacing PR title and description with...
Done. Thanks for the suggestion!
(https://github.com/bitcoin/bitcoin/pull/28774#issuecomment-1899991600)
> Would suggest replacing PR title and description with...
Done. Thanks for the suggestion!
💬 Olayiwolaxiii commented on issue "assumeutxo: nTx and nChainTx violations in CheckBlockIndex":
(https://github.com/bitcoin/bitcoin/issues/29261#issuecomment-1899994384)
> When disabling the "test-only" assumptions in CheckBlockIndex, the check fails. This is problematic, because test-only code should not affect the behavior of the program in production.
>
>
>
> Current diff:
>
>
>
> ```diff
>
> diff --git a/src/validation.cpp b/src/validation.cpp
>
> index 8c583c586c..00d7ee3736 100644
>
> --- a/src/validation.cpp
>
> +++ b/src/validation.cpp
>
> @@ -4866,9 +4866,9 @@ void ChainstateManager::CheckBlockIndex()
>
> unsigned int prev_chain_tx
...
(https://github.com/bitcoin/bitcoin/issues/29261#issuecomment-1899994384)
> When disabling the "test-only" assumptions in CheckBlockIndex, the check fails. This is problematic, because test-only code should not affect the behavior of the program in production.
>
>
>
> Current diff:
>
>
>
> ```diff
>
> diff --git a/src/validation.cpp b/src/validation.cpp
>
> index 8c583c586c..00d7ee3736 100644
>
> --- a/src/validation.cpp
>
> +++ b/src/validation.cpp
>
> @@ -4866,9 +4866,9 @@ void ChainstateManager::CheckBlockIndex()
>
> unsigned int prev_chain_tx
...
👍 delta1 approved a pull request: "test: p2p: check disconnect due to lack of desirable service flags"
(https://github.com/bitcoin/bitcoin/pull/29279#pullrequestreview-1831909216)
Code Review ACK 358561e54ef84f64b216ca8a4271ad17d91b1d51
Adds a new test, increases code coverage.
(https://github.com/bitcoin/bitcoin/pull/29279#pullrequestreview-1831909216)
Code Review ACK 358561e54ef84f64b216ca8a4271ad17d91b1d51
Adds a new test, increases code coverage.
💬 vasild commented on pull request "wallet: avoid returning a reference to vMasterKey after releasing the mutex that guards it":
(https://github.com/bitcoin/bitcoin/pull/28774#discussion_r1458625183)
Will do if I retouch.
(https://github.com/bitcoin/bitcoin/pull/28774#discussion_r1458625183)
Will do if I retouch.
💬 t-bast commented on pull request "v3 transaction policy for anti-pinning":
(https://github.com/bitcoin/bitcoin/pull/28948#issuecomment-1900036446)
> No you can’t spend in-mempool htlc outputs 1 CSV post-anchor. Assuming no 1 CSV, yes in my scenario.
We're analyzing this with the changes related to v3, in which we will remove the `CSV 1`.
> However attacker can construct revoked or valid pinning state to have the 483 outputs as inbound HTLC to inflate. Alice no preimage to spend them.
But Alice doesn't need the preimage to spend them, this is a revoked commitment, she spends through the revocation path. Unless you're thinking of a
...
(https://github.com/bitcoin/bitcoin/pull/28948#issuecomment-1900036446)
> No you can’t spend in-mempool htlc outputs 1 CSV post-anchor. Assuming no 1 CSV, yes in my scenario.
We're analyzing this with the changes related to v3, in which we will remove the `CSV 1`.
> However attacker can construct revoked or valid pinning state to have the 483 outputs as inbound HTLC to inflate. Alice no preimage to spend them.
But Alice doesn't need the preimage to spend them, this is a revoked commitment, she spends through the revocation path. Unless you're thinking of a
...
💬 Xaspr commented on issue "Unable to sync blockchain on laptop: ERROR: ReadBlockFromDisk: Deserialize or I/O error":
(https://github.com/bitcoin/bitcoin/issues/29255#issuecomment-1900056556)
Core just fully synced with the changes mentioned earlier:
> Reinstalled v26.0.0.
> Deleted antivirus software Bitdefender.
> Enabled Microsoft Defender, with an exclusion for the %appdata%/Bitcoin folder.
> Excluded bitcoin-qt for exploit mitigation in HitmanPro.Alert.
> Added the following lines in bitcoin.conf:
> connect=<<other node on LAN>>
> assumevalid=0
> dbcache=24000
> disablewallet=1
>
The issue wasn't caused by hardware, but must have been caused by antivirus softwa
...
(https://github.com/bitcoin/bitcoin/issues/29255#issuecomment-1900056556)
Core just fully synced with the changes mentioned earlier:
> Reinstalled v26.0.0.
> Deleted antivirus software Bitdefender.
> Enabled Microsoft Defender, with an exclusion for the %appdata%/Bitcoin folder.
> Excluded bitcoin-qt for exploit mitigation in HitmanPro.Alert.
> Added the following lines in bitcoin.conf:
> connect=<<other node on LAN>>
> assumevalid=0
> dbcache=24000
> disablewallet=1
>
The issue wasn't caused by hardware, but must have been caused by antivirus softwa
...
✅ Xaspr closed an issue: "Unable to sync blockchain on laptop: ERROR: ReadBlockFromDisk: Deserialize or I/O error"
(https://github.com/bitcoin/bitcoin/issues/29255)
(https://github.com/bitcoin/bitcoin/issues/29255)
⚠️ fanquake opened an issue: "Post startup stalling"
(https://github.com/bitcoin/bitcoin/issues/29281)
Noticed this on a node recently. Basically startup, sync two blocks, but then stall for 15 minutes, until we timed out a peer (in this case a blocks-only peer). @mzumsande mentioned this is because we were too close to tip, for the stalling logic to kick in, but also not close enough for the parallel download logic to kick in. I'm wondering if we should adjust the thresholds for either, to try and make this less likely to occur. It's possible this is also the same issue being seen here: https://
...
(https://github.com/bitcoin/bitcoin/issues/29281)
Noticed this on a node recently. Basically startup, sync two blocks, but then stall for 15 minutes, until we timed out a peer (in this case a blocks-only peer). @mzumsande mentioned this is because we were too close to tip, for the stalling logic to kick in, but also not close enough for the parallel download logic to kick in. I'm wondering if we should adjust the thresholds for either, to try and make this less likely to occur. It's possible this is also the same issue being seen here: https://
...