💬 brunoerg commented on pull request "docs: Improve `keypoolrefill` RPC docs":
(https://github.com/bitcoin/bitcoin/pull/32429#discussion_r2076427518)
Agreed. Otherwise, #28710 would have to drop it.
(https://github.com/bitcoin/bitcoin/pull/32429#discussion_r2076427518)
Agreed. Otherwise, #28710 would have to drop it.
👍 brunoerg approved a pull request: "fuzz: Remove unused TimeoutExpired catch in fuzz runner"
(https://github.com/bitcoin/bitcoin/pull/32388#pullrequestreview-2819842011)
code review ACK fa4804009ceba96926edd7f55ea22442ebdc665d
(https://github.com/bitcoin/bitcoin/pull/32388#pullrequestreview-2819842011)
code review ACK fa4804009ceba96926edd7f55ea22442ebdc665d
🚀 achow101 merged a pull request: "scripted-diff: adapt script error constant names in feature_taproot.py"
(https://github.com/bitcoin/bitcoin/pull/32415)
(https://github.com/bitcoin/bitcoin/pull/32415)
💬 esomore commented on pull request "Remove arbitrary limits on OP_Return (datacarrier) outputs":
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2856261607)
Now that Bitcoin Knots adoption is growing, it’s time to harden policy—enforce secp256k1 curve checks on bare pub keys to prevent invalid-key UTXO spam.
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2856261607)
Now that Bitcoin Knots adoption is growing, it’s time to harden policy—enforce secp256k1 curve checks on bare pub keys to prevent invalid-key UTXO spam.
🤔 w0xlt reviewed a pull request: "Remove the legacy wallet and BDB dependency"
(https://github.com/bitcoin/bitcoin/pull/28710#pullrequestreview-2819848971)
ACK https://github.com/bitcoin/bitcoin/pull/28710/commits/be3c13a22fa8d49fe15cde941e8030e1cd2b522f with 2 non-blocking nits commented earlier.
Regarding the removed RPC functons, I think less is better in this case, and unless I'm missing something, the cases mentioned in https://github.com/bitcoin/bitcoin/issues/30175 can be performed by `importdescriptors`.
(https://github.com/bitcoin/bitcoin/pull/28710#pullrequestreview-2819848971)
ACK https://github.com/bitcoin/bitcoin/pull/28710/commits/be3c13a22fa8d49fe15cde941e8030e1cd2b522f with 2 non-blocking nits commented earlier.
Regarding the removed RPC functons, I think less is better in this case, and unless I'm missing something, the cases mentioned in https://github.com/bitcoin/bitcoin/issues/30175 can be performed by `importdescriptors`.
📝 maflcko opened a pull request: "test: Add and use ElapseTime helper"
(https://github.com/bitcoin/bitcoin/pull/32430)
Currently mocktime is written to a global, which may leak between sub-tests (albeit some tests try to reset the mocktime on a best-effort basis). Also, when advancing it, one has to keep a counter variable around.
Fix both issues by a new `ElapseTime` helper, which resets the mocktime once it goes out of scope. Also, it has a method to advance the mocktime by a delta.
(https://github.com/bitcoin/bitcoin/pull/32430)
Currently mocktime is written to a global, which may leak between sub-tests (albeit some tests try to reset the mocktime on a best-effort basis). Also, when advancing it, one has to keep a counter variable around.
Fix both issues by a new `ElapseTime` helper, which resets the mocktime once it goes out of scope. Also, it has a method to advance the mocktime by a delta.
💬 RandyMcMillan commented on pull request "Remove arbitrary limits on OP_Return (datacarrier) outputs":
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2856341229)
ACK cd7872ca543d31d20f419fd2203138b8301c2e68
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2856341229)
ACK cd7872ca543d31d20f419fd2203138b8301c2e68
💬 pokrovskyy commented on pull request "Remove arbitrary limits on OP_Return (datacarrier) outputs":
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2856387463)
We all know we have Ordinals protocol that utilizes Taproot witness to store arbitrary data on-chain. Effectively permanently. And literally of any size (even beyond the block limits via chunking techniques). Regardless of the outcome of this conversation, this pathway will remain. Moreover, size-wise / fee-wise keeping bigger data in witness is 4x more blockspace-efficient and cheaper. So there's no incentive to choose OP_RETURN over witness data for significantly bigger data so should not incr
...
(https://github.com/bitcoin/bitcoin/pull/32359#issuecomment-2856387463)
We all know we have Ordinals protocol that utilizes Taproot witness to store arbitrary data on-chain. Effectively permanently. And literally of any size (even beyond the block limits via chunking techniques). Regardless of the outcome of this conversation, this pathway will remain. Moreover, size-wise / fee-wise keeping bigger data in witness is 4x more blockspace-efficient and cheaper. So there's no incentive to choose OP_RETURN over witness data for significantly bigger data so should not incr
...
💬 achow101 commented on pull request "miner: timelock the coinbase to the mined block's height":
(https://github.com/bitcoin/bitcoin/pull/32155#issuecomment-2856433009)
In 8f2078af6a55448c003b3f7f3021955fbb351caa "miner: timelock coinbase transactions"
I'm having a hard time understanding why `rbf_tests.cpp` and `mempool_package_rbf.py` are changed? These are not hardcoded txid changes. Rather the specific changes suggest to me that somehow the semantics of rbf have somehow changed? But that doesn't make sense if the only changes are to the coinbase transaction, why would that affect mempool policy?
(https://github.com/bitcoin/bitcoin/pull/32155#issuecomment-2856433009)
In 8f2078af6a55448c003b3f7f3021955fbb351caa "miner: timelock coinbase transactions"
I'm having a hard time understanding why `rbf_tests.cpp` and `mempool_package_rbf.py` are changed? These are not hardcoded txid changes. Rather the specific changes suggest to me that somehow the semantics of rbf have somehow changed? But that doesn't make sense if the only changes are to the coinbase transaction, why would that affect mempool policy?
💬 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.