💬 shahsb commented on pull request "(RFC) kernel: Replace leveldb-based BlockTreeDB with flat-file based store":
(https://github.com/bitcoin/bitcoin/pull/32427#issuecomment-2856877928)
10. On similar lines to point #5 -- **Corruption Detection** mechanism could also be implemented to detect corruptions very early in the cycle.
(https://github.com/bitcoin/bitcoin/pull/32427#issuecomment-2856877928)
10. On similar lines to point #5 -- **Corruption Detection** mechanism could also be implemented to detect corruptions very early in the cycle.
🤔 shahsb reviewed a pull request: "(RFC) kernel: Replace leveldb-based BlockTreeDB with flat-file based store"
(https://github.com/bitcoin/bitcoin/pull/32427#pullrequestreview-2820208053)
The flat-file approach is a reasonable trade-off given the simplicity of the block tree storage requirements.
However, there are major significant challenges and risks involved with this approach as highlighted in the above 10 comments. (Concurrency, Corruption/Integrity, Performance, Migration, Corruption detection, Portability, Backward compatibility etc.)
(https://github.com/bitcoin/bitcoin/pull/32427#pullrequestreview-2820208053)
The flat-file approach is a reasonable trade-off given the simplicity of the block tree storage requirements.
However, there are major significant challenges and risks involved with this approach as highlighted in the above 10 comments. (Concurrency, Corruption/Integrity, Performance, Migration, Corruption detection, Portability, Backward compatibility etc.)
💬 w0xlt commented on pull request "docs: Improve `keypoolrefill` RPC docs":
(https://github.com/bitcoin/bitcoin/pull/32429#discussion_r2076693232)
`OUTPUT_TYPES` handled in https://github.com/bitcoin/bitcoin/pull/32432
(https://github.com/bitcoin/bitcoin/pull/32429#discussion_r2076693232)
`OUTPUT_TYPES` handled in https://github.com/bitcoin/bitcoin/pull/32432
💬 ajtowns commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#discussion_r2076791280)
"multiple datacarrier outputs within a transaction"
(https://github.com/bitcoin/bitcoin/pull/32406#discussion_r2076791280)
"multiple datacarrier outputs within a transaction"
💬 Zodomo commented on pull request "Remove arbitrary limits on OP_Return (datacarrier) outputs":
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2857093636)
Concept ACK
The message Satoshi Nakamoto inscribed into the Genesis Block is 69 bytes, yet many advocate for reducing it to ~40. This change would have prevented Bitcoin's emergent criticism of the existing fiat system from being included. Satoshi stored "arbitrary data" in the very first block. To say that such usage is "spam" is to ignore Bitcoin's origin.
The world's most neutral and immutable ledger should be unopinionated towards what the free market would like, and pays for it to include
...
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2857093636)
Concept ACK
The message Satoshi Nakamoto inscribed into the Genesis Block is 69 bytes, yet many advocate for reducing it to ~40. This change would have prevented Bitcoin's emergent criticism of the existing fiat system from being included. Satoshi stored "arbitrary data" in the very first block. To say that such usage is "spam" is to ignore Bitcoin's origin.
The world's most neutral and immutable ledger should be unopinionated towards what the free market would like, and pays for it to include
...
💬 laanwj commented on pull request "deps: Bump lief to 0.16.5":
(https://github.com/bitcoin/bitcoin/pull/32431#issuecomment-2857144478)
Concept ACK.
> Fixes an existing bug, where bcrypt.dll was not one of the expected symbols in contrib/devtools/symbol-check.py, even though this symbol gets included by way of secp256k1:
From what i understand, that's only used for the example, not for the library itself. The bcrypt-using code (`fill_random` in `examples/examples_util.h`) is never included in the library.
It's unlikely that the LIEF version changes anything there.
(https://github.com/bitcoin/bitcoin/pull/32431#issuecomment-2857144478)
Concept ACK.
> Fixes an existing bug, where bcrypt.dll was not one of the expected symbols in contrib/devtools/symbol-check.py, even though this symbol gets included by way of secp256k1:
From what i understand, that's only used for the example, not for the library itself. The bcrypt-using code (`fill_random` in `examples/examples_util.h`) is never included in the library.
It's unlikely that the LIEF version changes anything there.
👍 laanwj approved a pull request: "random: Use modern Windows randomness functions"
(https://github.com/bitcoin/bitcoin/pull/32400#pullrequestreview-2820459818)
Code review ACK 345944bd6cf8be0c8b85b6d3ccb593ce26c598aa
Code changes look correct. i *haven't* tested this on an actual windows machine.
(https://github.com/bitcoin/bitcoin/pull/32400#pullrequestreview-2820459818)
Code review ACK 345944bd6cf8be0c8b85b6d3ccb593ce26c598aa
Code changes look correct. i *haven't* tested this on an actual windows machine.
🤔 hebasto reviewed a pull request: "deps: Bump lief to 0.16.5"
(https://github.com/bitcoin/bitcoin/pull/32431#pullrequestreview-2820462645)
Also the LIEF version must be bumped here: https://github.com/bitcoin/bitcoin/blob/59d3e4ed34eb55cb40928d524cb0bd5e183ed85a/contrib/guix/manifest.scm#L154-L161
(https://github.com/bitcoin/bitcoin/pull/32431#pullrequestreview-2820462645)
Also the LIEF version must be bumped here: https://github.com/bitcoin/bitcoin/blob/59d3e4ed34eb55cb40928d524cb0bd5e183ed85a/contrib/guix/manifest.scm#L154-L161
💬 AlexSQY commented on pull request "CAT in Tapscript (BIP-347)":
(https://github.com/bitcoin/bitcoin/pull/29247#issuecomment-2857183685)
What about SCRIPT_VERIFY OP_CAT = False and SCRIPT_VERIFY DISCOURAGE_OP_CAT = False?
This combination is not described in the "soft fork" table. Is it part of the tests?
(https://github.com/bitcoin/bitcoin/pull/29247#issuecomment-2857183685)
What about SCRIPT_VERIFY OP_CAT = False and SCRIPT_VERIFY DISCOURAGE_OP_CAT = False?
This combination is not described in the "soft fork" table. Is it part of the tests?
💬 hebasto commented on pull request "deps: Bump lief to 0.16.5":
(https://github.com/bitcoin/bitcoin/pull/32431#discussion_r2076868608)
Does Guix build succeed?
(https://github.com/bitcoin/bitcoin/pull/32431#discussion_r2076868608)
Does Guix build succeed?
💬 hebasto commented on pull request "deps: Bump lief to 0.16.5":
(https://github.com/bitcoin/bitcoin/pull/32431#issuecomment-2857203503)
Concept ACK.
(https://github.com/bitcoin/bitcoin/pull/32431#issuecomment-2857203503)
Concept ACK.
💬 davidgumberg commented on pull request "deps: Bump lief to 0.16.5":
(https://github.com/bitcoin/bitcoin/pull/32431#discussion_r2076875091)
In the process of testing this, I'll mark as draft in the meanwhile.
(https://github.com/bitcoin/bitcoin/pull/32431#discussion_r2076875091)
In the process of testing this, I'll mark as draft in the meanwhile.
💬 davidgumberg commented on pull request "deps: Bump lief to 0.16.5":
(https://github.com/bitcoin/bitcoin/pull/32431#issuecomment-2857251032)
> From what i understand, that's only used for the example, not for the library itself. The bcrypt-using code (`fill_random` in `examples/examples_util.h`) is never included in the library.
>
> It's unlikely that the LIEF version changes anything there.
Oops, my bad, I must have ran `symbol-check.py` against a stale build of the `bcrypt` branch, rebased to drop adding `bcrypt` to `symbol-check.py`, marking draft until I test this as working in Guix.
(https://github.com/bitcoin/bitcoin/pull/32431#issuecomment-2857251032)
> From what i understand, that's only used for the example, not for the library itself. The bcrypt-using code (`fill_random` in `examples/examples_util.h`) is never included in the library.
>
> It's unlikely that the LIEF version changes anything there.
Oops, my bad, I must have ran `symbol-check.py` against a stale build of the `bcrypt` branch, rebased to drop adding `bcrypt` to `symbol-check.py`, marking draft until I test this as working in Guix.
📝 davidgumberg converted_to_draft a pull request: "deps: Bump lief to 0.16.5"
(https://github.com/bitcoin/bitcoin/pull/32431)
Partially resolves https://github.com/bitcoin/bitcoin/issues/30520
1. Fixes an existing bug, where `bcrypt.dll` was not one of the expected symbols in `contrib/devtools/symbol-check.py`, even though this symbol gets included by way of `secp256k1`:https://github.com/bitcoin/bitcoin/blob/44057fe38ccc5d05a6249413467e002db2ae023d/src/secp256k1/examples/CMakeLists.txt#L9
2. Updates the version of the `lief` package to 0.16.5
3. Add usage notes for `contrib/devtools/security-check.py`
(https://github.com/bitcoin/bitcoin/pull/32431)
Partially resolves https://github.com/bitcoin/bitcoin/issues/30520
1. Fixes an existing bug, where `bcrypt.dll` was not one of the expected symbols in `contrib/devtools/symbol-check.py`, even though this symbol gets included by way of `secp256k1`:https://github.com/bitcoin/bitcoin/blob/44057fe38ccc5d05a6249413467e002db2ae023d/src/secp256k1/examples/CMakeLists.txt#L9
2. Updates the version of the `lief` package to 0.16.5
3. Add usage notes for `contrib/devtools/security-check.py`
💬 1440000bytes commented on pull request "policy: uncap datacarrier by default":
(https://github.com/bitcoin/bitcoin/pull/32406#issuecomment-2857253356)
After reading all comment and reviewing code.
utACK https://github.com/bitcoin/bitcoin/pull/32406/commits/3b3fafdb1c7eca8010194d0063579d44d555d546
(https://github.com/bitcoin/bitcoin/pull/32406#issuecomment-2857253356)
After reading all comment and reviewing code.
utACK https://github.com/bitcoin/bitcoin/pull/32406/commits/3b3fafdb1c7eca8010194d0063579d44d555d546
💬 Antiscaliger commented on pull request "Remove arbitrary limits on OP_Return (datacarrier) outputs":
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2857253461)
For many years now, the OP_RETURN size limit has been a steady deterrent to permanent users/developers who can hype up another technology and clog the base of the canonical chain.
I believe that the current release of the OP_RETURN restriction is definitely not the right time and is an artificial disturbance that distracts from real proposals.
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2857253461)
For many years now, the OP_RETURN size limit has been a steady deterrent to permanent users/developers who can hype up another technology and clog the base of the canonical chain.
I believe that the current release of the OP_RETURN restriction is definitely not the right time and is an artificial disturbance that distracts from real proposals.
💬 hebasto commented on issue "Depends toolchain doesn't contain enough info to build from depends on a fresh NixOS install":
(https://github.com/bitcoin/bitcoin/issues/32428#issuecomment-2857297811)
> > The problem is caused by `set(CMAKE_FIND_USE_CMAKE_ENVIRONMENT_PATH OFF)` in the toolchain file:
> >
> > bitcoin/depends/toolchain.cmake.in](https://github.com/bitcoin/bitcoin/blob/baa848b8d38187ce6b24a57cfadf1ea180209711/depends/toolchain.cmake.in#L86
> >
> > and I believe it goes away if you remove that line. I commented on this previously in [#30940 (review)](https://github.com/bitcoin/bitcoin/pull/30940#pullrequestreview-2322355196)
>
> Here is some additional context on how
...
(https://github.com/bitcoin/bitcoin/issues/32428#issuecomment-2857297811)
> > The problem is caused by `set(CMAKE_FIND_USE_CMAKE_ENVIRONMENT_PATH OFF)` in the toolchain file:
> >
> > bitcoin/depends/toolchain.cmake.in](https://github.com/bitcoin/bitcoin/blob/baa848b8d38187ce6b24a57cfadf1ea180209711/depends/toolchain.cmake.in#L86
> >
> > and I believe it goes away if you remove that line. I commented on this previously in [#30940 (review)](https://github.com/bitcoin/bitcoin/pull/30940#pullrequestreview-2322355196)
>
> Here is some additional context on how
...
💬 hebasto commented on issue "Depends toolchain doesn't contain enough info to build from depends on a fresh NixOS install":
(https://github.com/bitcoin/bitcoin/issues/32428#issuecomment-2857343533)
cc @purpleKarrot
(https://github.com/bitcoin/bitcoin/issues/32428#issuecomment-2857343533)
cc @purpleKarrot
⚠️ amg1127 opened an issue: ""rpcallowip=" configuration directive doesn't accept RFC4193 addresses"
(https://github.com/bitcoin/bitcoin/issues/32433)
### Is there an existing issue for this?
- [x] I have searched the existing issues
### Current behaviour
I have a local dual-stack network using RFC1918 addresses on IPv4 stack and RFC4193 addresses on IPv6 stack. I would like to restrict the access to RPC ports of Bitcoin Core nodes running in the network based on the source address of the connections. However, Bitcoin Core 29.0 doesn't accept `rpcallowip=` directives that specify addresses under the `[fc00::/7]` IP block.
### Expected beha
...
(https://github.com/bitcoin/bitcoin/issues/32433)
### Is there an existing issue for this?
- [x] I have searched the existing issues
### Current behaviour
I have a local dual-stack network using RFC1918 addresses on IPv4 stack and RFC4193 addresses on IPv6 stack. I would like to restrict the access to RPC ports of Bitcoin Core nodes running in the network based on the source address of the connections. However, Bitcoin Core 29.0 doesn't accept `rpcallowip=` directives that specify addresses under the `[fc00::/7]` IP block.
### Expected beha
...
💬 i5hi commented on pull request "Remove arbitrary limits on OP_Return (datacarrier) outputs":
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2857453312)
> > As a node runner I should be allowed to configure my mempool and not have it filled with what I consider spam.
>
> @i5hi what is the use case for running a mempool at all then?
Filtering spam* doesn't change the usecase of running a mempool. Broadcasting and relaying transactions (that I care about) so they reach miners.
spam* - being subjective since we all seem to have differing opinions on what spam is.
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2857453312)
> > As a node runner I should be allowed to configure my mempool and not have it filled with what I consider spam.
>
> @i5hi what is the use case for running a mempool at all then?
Filtering spam* doesn't change the usecase of running a mempool. Broadcasting and relaying transactions (that I care about) so they reach miners.
spam* - being subjective since we all seem to have differing opinions on what spam is.