🚀 glozow merged a pull request: "Update libsecp256k1 subtree to current master"
(https://github.com/bitcoin/bitcoin/pull/29169)
(https://github.com/bitcoin/bitcoin/pull/29169)
📝 theuni opened a pull request: "crypto: remove use of BUILD_BITCOIN_INTERNAL macro in sha256"
(https://github.com/bitcoin/bitcoin/pull/29180)
Replace it with a more explicit DISABLE_OPTIMIZED_SHA256 and clean up some.
The macro was originally only used by libbitcoinconsensus which opts out of optimized sha256 for the sake of simplicity. But now that libbitcoinkernel uses it as well, we need more fine-grained control.
As a result of this change, libbitcoinkernel's sha256 usage should now be optimized.
(https://github.com/bitcoin/bitcoin/pull/29180)
Replace it with a more explicit DISABLE_OPTIMIZED_SHA256 and clean up some.
The macro was originally only used by libbitcoinconsensus which opts out of optimized sha256 for the sake of simplicity. But now that libbitcoinkernel uses it as well, we need more fine-grained control.
As a result of this change, libbitcoinkernel's sha256 usage should now be optimized.
💬 theuni commented on pull request "crypto: remove use of BUILD_BITCOIN_INTERNAL macro in sha256":
(https://github.com/bitcoin/bitcoin/pull/29180#issuecomment-1877480455)
Ping @TheCharlatan @hebasto
I went around in circles several times on this. I think it accomplishes what we need, but please sanity check me :)
(https://github.com/bitcoin/bitcoin/pull/29180#issuecomment-1877480455)
Ping @TheCharlatan @hebasto
I went around in circles several times on this. I think it accomplishes what we need, but please sanity check me :)
💬 hebasto commented on pull request "crypto: remove use of BUILD_BITCOIN_INTERNAL macro in sha256":
(https://github.com/bitcoin/bitcoin/pull/29180#discussion_r1442055466)
Is it needed to keep `-DBUILD_BITCOIN_INTERNAL` now?
(https://github.com/bitcoin/bitcoin/pull/29180#discussion_r1442055466)
Is it needed to keep `-DBUILD_BITCOIN_INTERNAL` now?
💬 theuni commented on pull request "crypto: remove use of BUILD_BITCOIN_INTERNAL macro in sha256":
(https://github.com/bitcoin/bitcoin/pull/29180#discussion_r1442056422)
Yes, this macro is now exclusively used to control exports.
(https://github.com/bitcoin/bitcoin/pull/29180#discussion_r1442056422)
Yes, this macro is now exclusively used to control exports.
💬 furszy commented on issue "Wallets should update key/descriptor birthdates when txs older than current birthdates are found":
(https://github.com/bitcoin/bitcoin/issues/28897#issuecomment-1877494923)
Can be closed.
(https://github.com/bitcoin/bitcoin/issues/28897#issuecomment-1877494923)
Can be closed.
✅ maflcko closed an issue: "Wallets should update key/descriptor birthdates when txs older than current birthdates are found"
(https://github.com/bitcoin/bitcoin/issues/28897)
(https://github.com/bitcoin/bitcoin/issues/28897)
📝 fanquake opened a pull request: "build: remove systemtap variadic patch"
(https://github.com/bitcoin/bitcoin/pull/29181)
We now use C++20.
(https://github.com/bitcoin/bitcoin/pull/29181)
We now use C++20.
💬 theuni commented on pull request "crypto: remove use of BUILD_BITCOIN_INTERNAL macro in sha256":
(https://github.com/bitcoin/bitcoin/pull/29180#discussion_r1442077985)
Updated for @fanquake's request: it's now exclusively used to control libbitcoinconsensus exports. libbitcoinkernel will get a new define when the time comes.
(https://github.com/bitcoin/bitcoin/pull/29180#discussion_r1442077985)
Updated for @fanquake's request: it's now exclusively used to control libbitcoinconsensus exports. libbitcoinkernel will get a new define when the time comes.
📝 L0laL33tz opened a pull request: "test: randomize perturbed files excluding ldb"
(https://github.com/bitcoin/bitcoin/pull/29182)
As discussed in #28831 the ldb files are too small to be randomized for pertubation. This PR excludes ldb files from randomization. Blockfiles are randomly perturbed, ldb file pertubation remains deterministic. For additional rationale see #28612
(https://github.com/bitcoin/bitcoin/pull/29182)
As discussed in #28831 the ldb files are too small to be randomized for pertubation. This PR excludes ldb files from randomization. Blockfiles are randomly perturbed, ldb file pertubation remains deterministic. For additional rationale see #28612
🤔 ismaelsadeeq reviewed a pull request: "rpc: validate fee estimation mode case insensitive"
(https://github.com/bitcoin/bitcoin/pull/29175#pullrequestreview-1804697537)
Concept ACK
Thanks for trying to solve this issue.
This patch does not fix the issue.
Behaviour on master 65c05db660b2ca1d0076b0d8573a6760b3228068
```terminal
abubakarismail@Abubakars-MacBook-Pro bitcoin % ./src/bitcoind -regtest -fallbackfee=0.0000100 -daemon
Bitcoin Core starting
abubakarismail@Abubakars-MacBook-Pro bitcoin % ./src/bitcoin-cli -regtest loadwallet abubakar-test
{
"name": "abubakar-test"
}
abubakarismail@Abubakars-MacBook-Pro bitcoin % ./src/bitcoin-cli -reg
...
(https://github.com/bitcoin/bitcoin/pull/29175#pullrequestreview-1804697537)
Concept ACK
Thanks for trying to solve this issue.
This patch does not fix the issue.
Behaviour on master 65c05db660b2ca1d0076b0d8573a6760b3228068
```terminal
abubakarismail@Abubakars-MacBook-Pro bitcoin % ./src/bitcoind -regtest -fallbackfee=0.0000100 -daemon
Bitcoin Core starting
abubakarismail@Abubakars-MacBook-Pro bitcoin % ./src/bitcoin-cli -regtest loadwallet abubakar-test
{
"name": "abubakar-test"
}
abubakarismail@Abubakars-MacBook-Pro bitcoin % ./src/bitcoin-cli -reg
...
💬 theuni commented on pull request "refactor: C++20: Use std::rotl":
(https://github.com/bitcoin/bitcoin/pull/29085#issuecomment-1877527471)
Any reason not to do `Rotl` in sha3 and `ROTL` in siphash as well?
(https://github.com/bitcoin/bitcoin/pull/29085#issuecomment-1877527471)
Any reason not to do `Rotl` in sha3 and `ROTL` in siphash as well?
💬 jonatack commented on pull request "Add OP_TXHASH and OP_CHECKTXHASHVERIFY opcodes":
(https://github.com/bitcoin/bitcoin/pull/29050#discussion_r1442123784)
A working build and green CI may help attract review. The last commit https://github.com/bitcoin/bitcoin/pull/29050/commits/c368d32588ff79efc189fbdcfe559d1ca081c36c doesn't build for me without updating the related fuzz tests.
<details><summary>sample diff</summary><p>
```diff
diff --git a/src/test/fuzz/script_assets_test_minimizer.cpp b/src/test/fuzz/script_assets_test_minimizer.cpp
index 511b581f606..7478897e170 100644
--- a/src/test/fuzz/script_assets_test_minimizer.cpp
+++ b/src/t
...
(https://github.com/bitcoin/bitcoin/pull/29050#discussion_r1442123784)
A working build and green CI may help attract review. The last commit https://github.com/bitcoin/bitcoin/pull/29050/commits/c368d32588ff79efc189fbdcfe559d1ca081c36c doesn't build for me without updating the related fuzz tests.
<details><summary>sample diff</summary><p>
```diff
diff --git a/src/test/fuzz/script_assets_test_minimizer.cpp b/src/test/fuzz/script_assets_test_minimizer.cpp
index 511b581f606..7478897e170 100644
--- a/src/test/fuzz/script_assets_test_minimizer.cpp
+++ b/src/t
...
🤔 1ma reviewed a pull request: "datacarriersize: Match more datacarrying"
(https://github.com/bitcoin/bitcoin/pull/28408#pullrequestreview-1804807600)
In order to be consistent with the documentation of `datacarriersize`, this PR needs to roll back the changes recently made in `src/init.cpp` by this other PR: https://github.com/bitcoin/bitcoin/pull/27832
Not sure about the tests it added.
(https://github.com/bitcoin/bitcoin/pull/28408#pullrequestreview-1804807600)
In order to be consistent with the documentation of `datacarriersize`, this PR needs to roll back the changes recently made in `src/init.cpp` by this other PR: https://github.com/bitcoin/bitcoin/pull/27832
Not sure about the tests it added.
🤔 furszy reviewed a pull request: "Compressed Bitcoin Transactions"
(https://github.com/bitcoin/bitcoin/pull/29134#pullrequestreview-1804829839)
You could run the lint whitespace task locally. It is inside test/lint folder.
(https://github.com/bitcoin/bitcoin/pull/29134#pullrequestreview-1804829839)
You could run the lint whitespace task locally. It is inside test/lint folder.
💬 0xB10C commented on pull request "build: remove systemtap variadic patch":
(https://github.com/bitcoin/bitcoin/pull/29181#issuecomment-1877675266)
ACK. That patch isn't needed anymore.
(https://github.com/bitcoin/bitcoin/pull/29181#issuecomment-1877675266)
ACK. That patch isn't needed anymore.
💬 brunoerg commented on pull request "fuzz: set `nMaxOutboundLimit` in connman target":
(https://github.com/bitcoin/bitcoin/pull/29172#issuecomment-1877676278)
friendly ping: @dergoegge
(https://github.com/bitcoin/bitcoin/pull/29172#issuecomment-1877676278)
friendly ping: @dergoegge
💬 dzyphr commented on pull request "datacarriersize: Match more datacarrying":
(https://github.com/bitcoin/bitcoin/pull/28408#issuecomment-1877684914)
> > > Concept ACK
> > > The transactions targeted by this pull-req. are a very significant source of prohibitive cost for regular node operators. It is very unlikely that regular node operators will give up mitigating that source of operative cost. Regular policy rule for these transactions would encourage the economical resource usage of mempools - not harming any miners - neither changing any fee estimation already on the field.
> >
> >
> > As long as miners continue to mine these transa
...
(https://github.com/bitcoin/bitcoin/pull/28408#issuecomment-1877684914)
> > > Concept ACK
> > > The transactions targeted by this pull-req. are a very significant source of prohibitive cost for regular node operators. It is very unlikely that regular node operators will give up mitigating that source of operative cost. Regular policy rule for these transactions would encourage the economical resource usage of mempools - not harming any miners - neither changing any fee estimation already on the field.
> >
> >
> > As long as miners continue to mine these transa
...
💬 achow101 commented on pull request "wallet: `addhdkey` RPC to add just keys to wallets via new `void(KEY)` descriptor":
(https://github.com/bitcoin/bitcoin/pull/29136#issuecomment-1877688376)
> 1 - rename "void(KEY)" to "unused(KEY)"
> 3 - add logic to `importdescriptors` to disallow importing an unused(KEY) if KEY is used by another descriptor.
Done these. I've also added a further restriction that unused() cannot be import to a wallet without private keys. It isn't useful in such wallets so I think it makes sense to disallow their import.
> 2 - add logic to `importdescriptors` and `generatewalletdescriptor` to delete any unused(KEY) descriptor as soon as the KEY which it r
...
(https://github.com/bitcoin/bitcoin/pull/29136#issuecomment-1877688376)
> 1 - rename "void(KEY)" to "unused(KEY)"
> 3 - add logic to `importdescriptors` to disallow importing an unused(KEY) if KEY is used by another descriptor.
Done these. I've also added a further restriction that unused() cannot be import to a wallet without private keys. It isn't useful in such wallets so I think it makes sense to disallow their import.
> 2 - add logic to `importdescriptors` and `generatewalletdescriptor` to delete any unused(KEY) descriptor as soon as the KEY which it r
...
💬 hebasto commented on pull request "crypto: remove use of BUILD_BITCOIN_INTERNAL macro in sha256":
(https://github.com/bitcoin/bitcoin/pull/29180#issuecomment-1877692626)
Concept ACK.
It makes the `libbitcoinkernel` code equivalent for both internal and external usage.
(https://github.com/bitcoin/bitcoin/pull/29180#issuecomment-1877692626)
Concept ACK.
It makes the `libbitcoinkernel` code equivalent for both internal and external usage.