π¬ achow101 commented on issue "Moving this repo to bitcoin-core":
(https://github.com/bitcoin/bitcoin/issues/32340#issuecomment-2856469708)
A few people (@darosior, @glozow) mentioned at last week's IRC meeting that they'd like to have more time to think on this and we'd punt another week. If you have any thoughts on this topic that you'd like to share, that'd be appreciated. I'd rather not punt for another week again.
(https://github.com/bitcoin/bitcoin/issues/32340#issuecomment-2856469708)
A few people (@darosior, @glozow) mentioned at last week's IRC meeting that they'd like to have more time to think on this and we'd punt another week. If you have any thoughts on this topic that you'd like to share, that'd be appreciated. I'd rather not punt for another week again.
π murchandamus's pull request is ready for review: "coinselection: Optimize BnB exploration"
(https://github.com/bitcoin/bitcoin/pull/32150)
(https://github.com/bitcoin/bitcoin/pull/32150)
π¬ w0xlt commented on pull request "docs: Improve `keypoolrefill` RPC docs":
(https://github.com/bitcoin/bitcoin/pull/32429#discussion_r2076544489)
Done (for `DEFAULT_KEYPOOL_SIZE` and `ScriptPubKeyManagers`).
Regarding `OUTPUT_TYPES`, I think it may need to be refactored to include it in the RPC documentation due to static initialization.
But for now, I left `keypoolrefill` exactly as `getnewaddress`.
(https://github.com/bitcoin/bitcoin/pull/32429#discussion_r2076544489)
Done (for `DEFAULT_KEYPOOL_SIZE` and `ScriptPubKeyManagers`).
Regarding `OUTPUT_TYPES`, I think it may need to be refactored to include it in the RPC documentation due to static initialization.
But for now, I left `keypoolrefill` exactly as `getnewaddress`.
π¬ w0xlt commented on pull request "docs: Improve `keypoolrefill` RPC docs":
(https://github.com/bitcoin/bitcoin/pull/32429#discussion_r2076544724)
Done. Thanks.
(https://github.com/bitcoin/bitcoin/pull/32429#discussion_r2076544724)
Done. Thanks.
π¬ achow101 commented on pull request "Remove the legacy wallet and BDB dependency":
(https://github.com/bitcoin/bitcoin/pull/28710#discussion_r2076553927)
Maybe for a followup.
(https://github.com/bitcoin/bitcoin/pull/28710#discussion_r2076553927)
Maybe for a followup.
π¬ achow101 commented on pull request "Remove the legacy wallet and BDB dependency":
(https://github.com/bitcoin/bitcoin/pull/28710#discussion_r2076555339)
Done
(https://github.com/bitcoin/bitcoin/pull/28710#discussion_r2076555339)
Done
π¬ redfearnk commented on pull request "Remove arbitrary limits on OP_Return (datacarrier) outputs":
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2856683876)
Why are you moderating comments that are acknowledging conflicts of interests with those making arguments in favor/against this pull request?
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2856683876)
Why are you moderating comments that are acknowledging conflicts of interests with those making arguments in favor/against this pull request?
π davidgumberg opened a pull request: "deps: Bump lief to 0.16.5"
(https://github.com/bitcoin/bitcoin/pull/32431)
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)
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`
π¬ davidgumberg commented on pull request "random: Use modern Windows randomness functions":
(https://github.com/bitcoin/bitcoin/pull/32400#discussion_r2076602608)
Done, thanks.
(https://github.com/bitcoin/bitcoin/pull/32400#discussion_r2076602608)
Done, thanks.
π¬ davidgumberg commented on issue "guix: update LIEF from 0.13.2 to 0.16.x":
(https://github.com/bitcoin/bitcoin/issues/30520#issuecomment-2856726776)
I've opened #32431 to partially address this issue, it only bumps the version to 0.16.5 without implementing the other suggested improvements here.
(https://github.com/bitcoin/bitcoin/issues/30520#issuecomment-2856726776)
I've opened #32431 to partially address this issue, it only bumps the version to 0.16.5 without implementing the other suggested improvements here.
π¬ davidgumberg commented on pull request "random: Use modern Windows randomness functions":
(https://github.com/bitcoin/bitcoin/pull/32400#discussion_r2076607982)
Fixed both issues, thanks.
Actually, it turns out `symbol-check.py` actually complains on master about bcrypt! That's because it gets included by secp, I've opened https://github.com/bitcoin/bitcoin/pull/32431 which includes the same change and also bumps the lief version, I'll rebase (if) whichever of these two PR's get merged first.
(https://github.com/bitcoin/bitcoin/pull/32400#discussion_r2076607982)
Fixed both issues, thanks.
Actually, it turns out `symbol-check.py` actually complains on master about bcrypt! That's because it gets included by secp, I've opened https://github.com/bitcoin/bitcoin/pull/32431 which includes the same change and also bumps the lief version, I'll rebase (if) whichever of these two PR's get merged first.
π¬ davidgumberg commented on pull request "random: Use modern Windows randomness functions":
(https://github.com/bitcoin/bitcoin/pull/32400#issuecomment-2856732190)
Rebased to address reviewer feedback.
(https://github.com/bitcoin/bitcoin/pull/32400#issuecomment-2856732190)
Rebased to address reviewer feedback.
π€ w0xlt reviewed a pull request: "Remove the legacy wallet and BDB dependency"
(https://github.com/bitcoin/bitcoin/pull/28710#pullrequestreview-2820087181)
reACK https://github.com/bitcoin/bitcoin/pull/28710/commits/de054df6dc32cbd8b127c6761d9c65d95025e08f
(https://github.com/bitcoin/bitcoin/pull/28710#pullrequestreview-2820087181)
reACK https://github.com/bitcoin/bitcoin/pull/28710/commits/de054df6dc32cbd8b127c6761d9c65d95025e08f
π€ dominickbrasileiro reviewed a pull request: "Remove arbitrary limits on OP_Return (datacarrier) outputs"
(https://github.com/bitcoin/bitcoin/pull/32359#pullrequestreview-2820134558)
NACK
I understand the reasoning behind removing the default cap, but it doesn't make sense to remove the `-datacarrier` and `-datacarriersize` config options. Node operators should be able to configure their mempools and filter out data they don't want to store.
If spam is the problem, we should address the spam filter mechanisms instead of simply allowing it to be stored somewhere else.
(https://github.com/bitcoin/bitcoin/pull/32359#pullrequestreview-2820134558)
NACK
I understand the reasoning behind removing the default cap, but it doesn't make sense to remove the `-datacarrier` and `-datacarriersize` config options. Node operators should be able to configure their mempools and filter out data they don't want to store.
If spam is the problem, we should address the spam filter mechanisms instead of simply allowing it to be stored somewhere else.
π w0xlt opened a pull request: "wallet, rpc: Change `OutputType` items from `string` into compile-time constants `string_view`"
(https://github.com/bitcoin/bitcoin/pull/32432)
Follow-up to https://github.com/bitcoin/bitcoin/pull/32429, built on top of it.
This PR addresses the https://github.com/bitcoin/bitcoin/pull/32429#discussion_r2076251627 that the RPC documentation does not use `OUTPUT_TYPES`, but rather hardcodes them, as is already the case for the `getnewaddress` command.
So here the output types are changed from `std::string` to `std::string_view` so that the values are known at compile time or during the early stages of program startup, before main() ex
...
(https://github.com/bitcoin/bitcoin/pull/32432)
Follow-up to https://github.com/bitcoin/bitcoin/pull/32429, built on top of it.
This PR addresses the https://github.com/bitcoin/bitcoin/pull/32429#discussion_r2076251627 that the RPC documentation does not use `OUTPUT_TYPES`, but rather hardcodes them, as is already the case for the `getnewaddress` command.
So here the output types are changed from `std::string` to `std::string_view` so that the values are known at compile time or during the early stages of program startup, before main() ex
...
π¬ shahsb commented on pull request "(RFC) kernel: Replace leveldb-based BlockTreeDB with flat-file based store":
(https://github.com/bitcoin/bitcoin/pull/32427#issuecomment-2856855525)
Thanks @TheCharlatan for this proposal and making the code changes.!
Please find my review comments, suggestions and some clarifying questions:
1) How will concurrent reads (and potentially writes) be handled with the flat file format?
2) Even if no deletion occurs, file corruption or partial writes can happen. Are you planning mmap or memory buffering?
3) What would be the "Corruption Recovery Strategy?" -- While the write-ahead log is mentioned as future work, providing even a mini
...
(https://github.com/bitcoin/bitcoin/pull/32427#issuecomment-2856855525)
Thanks @TheCharlatan for this proposal and making the code changes.!
Please find my review comments, suggestions and some clarifying questions:
1) How will concurrent reads (and potentially writes) be handled with the flat file format?
2) Even if no deletion occurs, file corruption or partial writes can happen. Are you planning mmap or memory buffering?
3) What would be the "Corruption Recovery Strategy?" -- While the write-ahead log is mentioned as future work, providing even a mini
...
π¬ shahsb commented on pull request "(RFC) kernel: Replace leveldb-based BlockTreeDB with flat-file based store":
(https://github.com/bitcoin/bitcoin/pull/32427#issuecomment-2856856921)
Consider including a pluggable interface that allows fallback to LevelDB for testing or backwards compatibility
(https://github.com/bitcoin/bitcoin/pull/32427#issuecomment-2856856921)
Consider including a pluggable interface that allows fallback to LevelDB for testing or backwards compatibility
π¬ samurai321 commented on pull request "Remove arbitrary limits on OP_Return (datacarrier) outputs":
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2856862207)
Concept NACK.
how about enabling an official way to store info about a transaction? And close the loopholes.
Just state that OP_RETURN should contain a hash at most and then query that info from an, optional to run, bittorrent DHT. (v2.0 )
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2856862207)
Concept NACK.
how about enabling an official way to store info about a transaction? And close the loopholes.
Just state that OP_RETURN should contain a hash at most and then query that info from an, optional to run, bittorrent DHT. (v2.0 )
π¬ shahsb commented on pull request "(RFC) kernel: Replace leveldb-based BlockTreeDB with flat-file based store":
(https://github.com/bitcoin/bitcoin/pull/32427#issuecomment-2856869458)
7. **Write contention and corruption risks:** -- While flat files avoid LevelDBβs process-level locking, concurrent writes require a mechanism (e.g., file locks, flock()) to prevent race conditions.
8. **Portability and Cross platform related edge cases: ** -- Ensure file-locking mechanisms (e.g., fcntl on Unix, LockFileEx on Windows) are robust.
(https://github.com/bitcoin/bitcoin/pull/32427#issuecomment-2856869458)
7. **Write contention and corruption risks:** -- While flat files avoid LevelDBβs process-level locking, concurrent writes require a mechanism (e.g., file locks, flock()) to prevent race conditions.
8. **Portability and Cross platform related edge cases: ** -- Ensure file-locking mechanisms (e.g., fcntl on Unix, LockFileEx on Windows) are robust.
π¬ shahsb commented on pull request "(RFC) kernel: Replace leveldb-based BlockTreeDB with flat-file based store":
(https://github.com/bitcoin/bitcoin/pull/32427#issuecomment-2856870786)
9. How about Handling Large Files? -- Test edge cases like file sizes approaching OS limits (e.g., 2+ GB on 32-bit systems). What would happen in such cases?
(https://github.com/bitcoin/bitcoin/pull/32427#issuecomment-2856870786)
9. How about Handling Large Files? -- Test edge cases like file sizes approaching OS limits (e.g., 2+ GB on 32-bit systems). What would happen in such cases?