💬 achow101 commented on pull request "wallet: Always rewrite tx records during migration":
(https://github.com/bitcoin/bitcoin/pull/32985#issuecomment-3512556430)
> these changes are persisted during `ReorderTransactions()`
Not for migration since legacy wallets are only opened with a `BerkeleyRODatabase` which does not write anything.
(https://github.com/bitcoin/bitcoin/pull/32985#issuecomment-3512556430)
> these changes are persisted during `ReorderTransactions()`
Not for migration since legacy wallets are only opened with a `BerkeleyRODatabase` which does not write anything.
💬 maflcko commented on pull request "build: Bump g++ minimum supported version to 12":
(https://github.com/bitcoin/bitcoin/pull/33842#issuecomment-3512575639)
> Such a change might also require updating the build guides. For instance, the following commands on Ubuntu 22.04 would use GCC 11:
I am happy to add a general note that the latest stable or LTS is recommended and that earlier releases may need the user to install the required minimum dependencies.
(https://github.com/bitcoin/bitcoin/pull/33842#issuecomment-3512575639)
> Such a change might also require updating the build guides. For instance, the following commands on Ubuntu 22.04 would use GCC 11:
I am happy to add a general note that the latest stable or LTS is recommended and that earlier releases may need the user to install the required minimum dependencies.
📝 nichechristie opened a pull request: "Claude/create coin 011 c uz rux6 kwdft yev8 a5 zsg"
(https://github.com/bitcoin/bitcoin/pull/33844)
<!--
*** Please remove the following help text before submitting: ***
Pull requests without a rationale and clear improvement may be closed
immediately.
GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->
<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:
* Any test improvements or new tests that improv
...
(https://github.com/bitcoin/bitcoin/pull/33844)
<!--
*** Please remove the following help text before submitting: ***
Pull requests without a rationale and clear improvement may be closed
immediately.
GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->
<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:
* Any test improvements or new tests that improv
...
✅ hebasto closed a pull request: "Claude/create coin 011 c uz rux6 kwdft yev8 a5 zsg"
(https://github.com/bitcoin/bitcoin/pull/33844)
(https://github.com/bitcoin/bitcoin/pull/33844)
🤔 janb84 reviewed a pull request: "kernel: trim Chain interface"
(https://github.com/bitcoin/bitcoin/pull/33820#pullrequestreview-3443942718)
ACK 66978a1a95379a2fe5d41032682dedfaddc99db9
Was already wondering why these 2 functions where present in the code. With the iteration ability I agree that there is no need for these "speciality" functions. The functionality can be implemented downstream if needed.
steps taken:
code review,
build,
ran all tests,
(small contemplation; chain.tip is 'often' used in the rest of the bitcoin code, is removing the `tip` function from the header not a performance burden downstream to ca
...
(https://github.com/bitcoin/bitcoin/pull/33820#pullrequestreview-3443942718)
ACK 66978a1a95379a2fe5d41032682dedfaddc99db9
Was already wondering why these 2 functions where present in the code. With the iteration ability I agree that there is no need for these "speciality" functions. The functionality can be implemented downstream if needed.
steps taken:
code review,
build,
ran all tests,
(small contemplation; chain.tip is 'often' used in the rest of the bitcoin code, is removing the `tip` function from the header not a performance burden downstream to ca
...
💬 janb84 commented on pull request "kernel: trim Chain interface":
(https://github.com/bitcoin/bitcoin/pull/33820#discussion_r2511078966)
```C
auto genesis_block_raw{chainman->ReadBlock(genesis_index).value().ToBytes()};
auto first_index{chain.GetByHeight(0)};
auto first_block_raw{chainman->ReadBlock(genesis_index).value().ToBytes()};
```
Not sure if this part of the test still makes sense, the double retrieval of the data of block 0 `auto first_block_raw{chainman->ReadBlock(genesis_index).value().ToBytes()};`
we are now testing if `chainman->ReadBlock` will retrieve the same block twice not if it will get the
...
(https://github.com/bitcoin/bitcoin/pull/33820#discussion_r2511078966)
```C
auto genesis_block_raw{chainman->ReadBlock(genesis_index).value().ToBytes()};
auto first_index{chain.GetByHeight(0)};
auto first_block_raw{chainman->ReadBlock(genesis_index).value().ToBytes()};
```
Not sure if this part of the test still makes sense, the double retrieval of the data of block 0 `auto first_block_raw{chainman->ReadBlock(genesis_index).value().ToBytes()};`
we are now testing if `chainman->ReadBlock` will retrieve the same block twice not if it will get the
...
💬 achow101 commented on pull request "log: reduce excessive "rolling back/forward" messages during block replay":
(https://github.com/bitcoin/bitcoin/pull/33443#issuecomment-3512621236)
ACK 1fc7a81f1f5f30ba3ebe305ac2a520c7b4afb596
(https://github.com/bitcoin/bitcoin/pull/33443#issuecomment-3512621236)
ACK 1fc7a81f1f5f30ba3ebe305ac2a520c7b4afb596
📝 maflcko opened a pull request: "ci: Enable experimental kernel stuff in ASan task"
(https://github.com/bitcoin/bitcoin/pull/33845)
Base the task on --preset=dev-mode to ensure maximal coverage and add the following:
```
bitcoin-chainstate (experimental) ... ON
libbitcoinkernel (experimental) ..... ON
kernel-test (experimental) .......... ON
```
Currently a draft to figure out https://github.com/bitcoin/bitcoin/pull/33824#issuecomment-3511082065
(https://github.com/bitcoin/bitcoin/pull/33845)
Base the task on --preset=dev-mode to ensure maximal coverage and add the following:
```
bitcoin-chainstate (experimental) ... ON
libbitcoinkernel (experimental) ..... ON
kernel-test (experimental) .......... ON
```
Currently a draft to figure out https://github.com/bitcoin/bitcoin/pull/33824#issuecomment-3511082065
🚀 achow101 merged a pull request: "log: reduce excessive "rolling back/forward" messages during block replay"
(https://github.com/bitcoin/bitcoin/pull/33443)
(https://github.com/bitcoin/bitcoin/pull/33443)
💬 maflcko commented on pull request "ci: Enable experimental kernel stuff in ASan task":
(https://github.com/bitcoin/bitcoin/pull/33845#issuecomment-3512701642)
Continue from https://github.com/bitcoin/bitcoin/pull/33824#issuecomment-3511082065:
> > I was thinking about permitting nullptr when size=0, or even just permitting nullptr for any size, when the data pointer is followed by a size?
>
> Afaict our de-serialization can handle this case (nullptr when size=0) correctly, but fails when passed a nullptr with non-zero size. Maybe a null check in `btck_block_create(...)` would be better? What do you think?
Yes, either should be fine. To clarif
...
(https://github.com/bitcoin/bitcoin/pull/33845#issuecomment-3512701642)
Continue from https://github.com/bitcoin/bitcoin/pull/33824#issuecomment-3511082065:
> > I was thinking about permitting nullptr when size=0, or even just permitting nullptr for any size, when the data pointer is followed by a size?
>
> Afaict our de-serialization can handle this case (nullptr when size=0) correctly, but fails when passed a nullptr with non-zero size. Maybe a null check in `btck_block_create(...)` would be better? What do you think?
Yes, either should be fine. To clarif
...
💬 maflcko commented on pull request "ci: Enable experimental kernel stuff in most CI tasks":
(https://github.com/bitcoin/bitcoin/pull/33824#issuecomment-3512704107)
Split the failing CI tasks out to https://github.com/bitcoin/bitcoin/pull/33845 and https://github.com/bitcoin/bitcoin/pull/33842
(https://github.com/bitcoin/bitcoin/pull/33824#issuecomment-3512704107)
Split the failing CI tasks out to https://github.com/bitcoin/bitcoin/pull/33845 and https://github.com/bitcoin/bitcoin/pull/33842
💬 achow101 commented on pull request "p2p: Advance pindexLastCommonBlock early after connecting blocks":
(https://github.com/bitcoin/bitcoin/pull/32180#issuecomment-3512710368)
ACK 01cc20f3307c532f402cdf5dc17f2f14920b725b
(https://github.com/bitcoin/bitcoin/pull/32180#issuecomment-3512710368)
ACK 01cc20f3307c532f402cdf5dc17f2f14920b725b
💬 kevkevinpal commented on pull request "test: Ignore error message give from python because of PYTHON_GIL":
(https://github.com/bitcoin/bitcoin/pull/33795#issuecomment-3512720950)
Added a TODO to remove the warning suppression once `capnp` can run safely without the GIL
[5b7778c](https://github.com/bitcoin/bitcoin/pull/33795/commits/5b7778ca6d00db3fef92384c373e67261abb65a0)
(https://github.com/bitcoin/bitcoin/pull/33795#issuecomment-3512720950)
Added a TODO to remove the warning suppression once `capnp` can run safely without the GIL
[5b7778c](https://github.com/bitcoin/bitcoin/pull/33795/commits/5b7778ca6d00db3fef92384c373e67261abb65a0)
✅ fanquake closed an issue: "log: "Replaying blocks" / "Rolling forward" logging is rate-limited"
(https://github.com/bitcoin/bitcoin/issues/33769)
(https://github.com/bitcoin/bitcoin/issues/33769)
💬 fanquake commented on issue "log: "Replaying blocks" / "Rolling forward" logging is rate-limited":
(https://github.com/bitcoin/bitcoin/issues/33769#issuecomment-3512779755)
Closing given #33443.
(https://github.com/bitcoin/bitcoin/issues/33769#issuecomment-3512779755)
Closing given #33443.
✅ achow101 closed an issue: "p2p: Inefficiency in block download / stalling algorithm"
(https://github.com/bitcoin/bitcoin/issues/32179)
(https://github.com/bitcoin/bitcoin/issues/32179)
🚀 achow101 merged a pull request: "p2p: Advance pindexLastCommonBlock early after connecting blocks"
(https://github.com/bitcoin/bitcoin/pull/32180)
(https://github.com/bitcoin/bitcoin/pull/32180)
💬 achow101 commented on pull request "rpc: add "ischange: true" to decoded tx outputs in wallet gettransaction response":
(https://github.com/bitcoin/bitcoin/pull/32517#issuecomment-3512800884)
ACK 060bb55508245776bb6a39c8b7849769ee588d69
(https://github.com/bitcoin/bitcoin/pull/32517#issuecomment-3512800884)
ACK 060bb55508245776bb6a39c8b7849769ee588d69
💬 achow101 commented on pull request "crypto: Use secure_allocator for `AES256_ctx`":
(https://github.com/bitcoin/bitcoin/pull/31774#discussion_r2511292626)
In 26442fec572da263765c576c080186c84cee1615 "bench: Add wallet encryption benchmark"
We've generally not had "measure overhead" in the benchmarks. If this is because encryption is actually a very small amount of time, I would suggest generating significantly more keys that need to actually be encrypted.
(https://github.com/bitcoin/bitcoin/pull/31774#discussion_r2511292626)
In 26442fec572da263765c576c080186c84cee1615 "bench: Add wallet encryption benchmark"
We've generally not had "measure overhead" in the benchmarks. If this is because encryption is actually a very small amount of time, I would suggest generating significantly more keys that need to actually be encrypted.
💬 achow101 commented on pull request "crypto: Use secure_allocator for `AES256_ctx`":
(https://github.com/bitcoin/bitcoin/pull/31774#discussion_r2511269902)
In 7176b26cde7cbaffdd92af9c25f85f8e5233e78a "refactor: Generalize derivation target calculation"
This comment does not seem helpful, I'm not sure what information it is trying to convey.
(https://github.com/bitcoin/bitcoin/pull/31774#discussion_r2511269902)
In 7176b26cde7cbaffdd92af9c25f85f8e5233e78a "refactor: Generalize derivation target calculation"
This comment does not seem helpful, I'm not sure what information it is trying to convey.