π¬ Sjors commented on pull request "mining: getCoinbase() returns struct instead of raw tx":
(https://github.com/bitcoin/bitcoin/pull/33819#discussion_r2584981169)
I renamed it to `block_reward_remaining`.
My understanding is that with merged-mining (see e.g. #33890) miners are expected to add their own outputs _after_ we give them a block template. These are typically 0 value `OP_RETURN` outputs, but - if the pool permits - they could be non-zero.
If we assume that nobody is going to patch Bitcoin Core to add outputs here (instead of downstream), then `block_reward` is accurate. For now...
However, there's still the possibility of the hypothetica
...
(https://github.com/bitcoin/bitcoin/pull/33819#discussion_r2584981169)
I renamed it to `block_reward_remaining`.
My understanding is that with merged-mining (see e.g. #33890) miners are expected to add their own outputs _after_ we give them a block template. These are typically 0 value `OP_RETURN` outputs, but - if the pool permits - they could be non-zero.
If we assume that nobody is going to patch Bitcoin Core to add outputs here (instead of downstream), then `block_reward` is accurate. For now...
However, there's still the possibility of the hypothetica
...
π¬ Sjors commented on pull request "mining: getCoinbase() returns struct instead of raw tx":
(https://github.com/bitcoin/bitcoin/pull/33819#discussion_r2584983479)
Taken, except:
- I used stronger wording for the 8 byte guarantee
- mention that OP_0 is historically an extranonce (which hopefully goes away, see https://github.com/bitcoin/bitcoin/pull/32420)
(https://github.com/bitcoin/bitcoin/pull/33819#discussion_r2584983479)
Taken, except:
- I used stronger wording for the 8 byte guarantee
- mention that OP_0 is historically an extranonce (which hopefully goes away, see https://github.com/bitcoin/bitcoin/pull/32420)
π¬ Sjors commented on pull request "mining: getCoinbase() returns struct instead of raw tx":
(https://github.com/bitcoin/bitcoin/pull/33819#discussion_r2584984317)
Added it back
(https://github.com/bitcoin/bitcoin/pull/33819#discussion_r2584984317)
Added it back
π¬ Sjors commented on pull request "mining: getCoinbase() returns struct instead of raw tx":
(https://github.com/bitcoin/bitcoin/pull/33819#discussion_r2584984800)
This should be better now.
(https://github.com/bitcoin/bitcoin/pull/33819#discussion_r2584984800)
This should be better now.
π¬ Sjors commented on pull request "mining: getCoinbase() returns struct instead of raw tx":
(https://github.com/bitcoin/bitcoin/pull/33819#issuecomment-3606701833)
I got rid of `ExtractCoinbaseTemplate` in favour of having `CreateNewBlock()` produce the `CoinbaseTemplate` right as it constructs the dummy template. It's then stored on the `CBlockTemplate` as suggested by @ryanofsky.
Since "coinbase" refers to the scriptSig of the coinbase _transaction_, I renamed some things:
- `CoinbaseTemplate` -> `CoinbaseTxTemplate`
- `getCoinbaseTx()` to `getCoinbaseRawTx()` (IPC clients are not affected by renames)
- `getCoinbase()` to `getCoinbaseTx()`
(https://github.com/bitcoin/bitcoin/pull/33819#issuecomment-3606701833)
I got rid of `ExtractCoinbaseTemplate` in favour of having `CreateNewBlock()` produce the `CoinbaseTemplate` right as it constructs the dummy template. It's then stored on the `CBlockTemplate` as suggested by @ryanofsky.
Since "coinbase" refers to the scriptSig of the coinbase _transaction_, I renamed some things:
- `CoinbaseTemplate` -> `CoinbaseTxTemplate`
- `getCoinbaseTx()` to `getCoinbaseRawTx()` (IPC clients are not affected by renames)
- `getCoinbase()` to `getCoinbaseTx()`
π hodlinator approved a pull request: "doc: Add `x86_64-w64-mingw32ucrt` triplet to `depends/README.md`"
(https://github.com/bitcoin/bitcoin/pull/33857#pullrequestreview-3534935501)
ACK ec8eb013a9bfceb324b309f13b8946b05292a993
Ran
`env -i HOME="$HOME" PATH="$PATH" USER="$USER" bash -c 'MAKEJOBS="-j$(nproc)" FILE_ENV="./ci/test/00_setup_env_win64.sh" ./ci/test_run_all.sh'`
locally and confirmed it successfully generated an executable at
`/ci_container_base/ci/scratch/build-x86_64-w64-mingw32ucrt/bin/bitcoind.exe`
(https://github.com/bitcoin/bitcoin/pull/33857#pullrequestreview-3534935501)
ACK ec8eb013a9bfceb324b309f13b8946b05292a993
Ran
`env -i HOME="$HOME" PATH="$PATH" USER="$USER" bash -c 'MAKEJOBS="-j$(nproc)" FILE_ENV="./ci/test/00_setup_env_win64.sh" ./ci/test_run_all.sh'`
locally and confirmed it successfully generated an executable at
`/ci_container_base/ci/scratch/build-x86_64-w64-mingw32ucrt/bin/bitcoind.exe`
π¬ fanquake commented on pull request "doc: Add `x86_64-w64-mingw32ucrt` triplet to `depends/README.md`":
(https://github.com/bitcoin/bitcoin/pull/33857#discussion_r2585077766)
> My hope was that one toolchain could be dropped while adding the new one relatively quickly, or even at the same time.
Yea. That was my intention/expectation for any migration.
(https://github.com/bitcoin/bitcoin/pull/33857#discussion_r2585077766)
> My hope was that one toolchain could be dropped while adding the new one relatively quickly, or even at the same time.
Yea. That was my intention/expectation for any migration.
π¬ fanquake commented on pull request "doc: Add `x86_64-w64-mingw32ucrt` triplet to `depends/README.md`":
(https://github.com/bitcoin/bitcoin/pull/33857#discussion_r2585095510)
> developers on Ubuntu, for example, may still want to build for x86_64-w64-mingw32.
Are we supporting that, if the CI is removed in #33593 (along with dropping workarounds going forward).
(https://github.com/bitcoin/bitcoin/pull/33857#discussion_r2585095510)
> developers on Ubuntu, for example, may still want to build for x86_64-w64-mingw32.
Are we supporting that, if the CI is removed in #33593 (along with dropping workarounds going forward).
π¬ maflcko commented on pull request "wallet: Be able to receive and spend inputs involving MuSig2 aggregate keys":
(https://github.com/bitcoin/bitcoin/pull/29675#discussion_r2585106930)
looks like this line crashes:
```
$ echo 'cHNidP8BAP0+AQIAAAAGy4dxGNHKAQgFPtEAoVgyMqb+N0ghgmdne5OjULYs2sUAAAr/AP3////L
fnEY0crZmwU+EVahszExpv43SCGCZ2d7k6NQtizaOwADAC4ABHwdAMt+cRjRytmbBT4RVqFYMTGm
/jdIIYJnZ3uTo1AAOwAsAAC2KwD9////hqbXlTl39C5ZmAtOAI4+zy32vWLVnyQjw7IvQsBa6rYA
AAcAAP3//zKG/zqgb7JIwmli68xeUjUXVp+c4b/tvc55IsAI4yQAAAAAAAAA/f///8t/0cpxGNmb
BT4RVqFYMTOm/jdIIXj///8AAAAAgNo7AACAAAD9//3/AiHf9QUAAAAAFgAU9/Ykq9yiCFGnUpsi
RsS6FFGLkQEAAAAAAAABABYAFEEQliEApQAADCBABv78gsn+/////wAAAAAAAAAAACIagHQA
...
(https://github.com/bitcoin/bitcoin/pull/29675#discussion_r2585106930)
looks like this line crashes:
```
$ echo 'cHNidP8BAP0+AQIAAAAGy4dxGNHKAQgFPtEAoVgyMqb+N0ghgmdne5OjULYs2sUAAAr/AP3////L
fnEY0crZmwU+EVahszExpv43SCGCZ2d7k6NQtizaOwADAC4ABHwdAMt+cRjRytmbBT4RVqFYMTGm
/jdIIYJnZ3uTo1AAOwAsAAC2KwD9////hqbXlTl39C5ZmAtOAI4+zy32vWLVnyQjw7IvQsBa6rYA
AAcAAP3//zKG/zqgb7JIwmli68xeUjUXVp+c4b/tvc55IsAI4yQAAAAAAAAA/f///8t/0cpxGNmb
BT4RVqFYMTOm/jdIIXj///8AAAAAgNo7AACAAAD9//3/AiHf9QUAAAAAFgAU9/Ykq9yiCFGnUpsi
RsS6FFGLkQEAAAAAAAABABYAFEEQliEApQAADCBABv78gsn+/////wAAAAAAAAAAACIagHQA
...
β οΈ maflcko opened an issue: "FUZZ=psbt in musig2, runs into uninitialized read"
(https://github.com/bitcoin/bitcoin/issues/33999)
```
$ echo 'cHNidP8BAP0+AQIAAAAGy4dxGNHKAQgFPtEAoVgyMqb+N0ghgmdne5OjULYs2sUAAAr/AP3////L
fnEY0crZmwU+EVahszExpv43SCGCZ2d7k6NQtizaOwADAC4ABHwdAMt+cRjRytmbBT4RVqFYMTGm
/jdIIYJnZ3uTo1AAOwAsAAC2KwD9////hqbXlTl39C5ZmAtOAI4+zy32vWLVnyQjw7IvQsBa6rYA
AAcAAP3//zKG/zqgb7JIwmli68xeUjUXVp+c4b/tvc55IsAI4yQAAAAAAAAA/f///8t/0cpxGNmb
BT4RVqFYMTOm/jdIIXj///8AAAAAgNo7AACAAAD9//3/AiHf9QUAAAAAFgAU9/Ykq9yiCFGnUpsi
RsS6FFGLkQEAAAAAAAABABYAFEEQliEApQAADCBABv78gsn+/////wAAAAAAAAAAACIagHQAAAAV
DLQyJycoAHNiKDIDAEIAAEEA
...
(https://github.com/bitcoin/bitcoin/issues/33999)
```
$ echo 'cHNidP8BAP0+AQIAAAAGy4dxGNHKAQgFPtEAoVgyMqb+N0ghgmdne5OjULYs2sUAAAr/AP3////L
fnEY0crZmwU+EVahszExpv43SCGCZ2d7k6NQtizaOwADAC4ABHwdAMt+cRjRytmbBT4RVqFYMTGm
/jdIIYJnZ3uTo1AAOwAsAAC2KwD9////hqbXlTl39C5ZmAtOAI4+zy32vWLVnyQjw7IvQsBa6rYA
AAcAAP3//zKG/zqgb7JIwmli68xeUjUXVp+c4b/tvc55IsAI4yQAAAAAAAAA/f///8t/0cpxGNmb
BT4RVqFYMTOm/jdIIXj///8AAAAAgNo7AACAAAD9//3/AiHf9QUAAAAAFgAU9/Ykq9yiCFGnUpsi
RsS6FFGLkQEAAAAAAAABABYAFEEQliEApQAADCBABv78gsn+/////wAAAAAAAAAAACIagHQAAAAV
DLQyJycoAHNiKDIDAEIAAEEA
...
β οΈ fanquake opened an issue: "fuzz: crash in psbt fuzzer"
(https://github.com/bitcoin/bitcoin/issues/34000)
From: https://github.com/bitcoin/bitcoin/pull/29675#discussion_r2585106930:
> looks like this line crashes:
```bash
$ echo 'cHNidP8BAP0+AQIAAAAGy4dxGNHKAQgFPtEAoVgyMqb+N0ghgmdne5OjULYs2sUAAAr/AP3////L
fnEY0crZmwU+EVahszExpv43SCGCZ2d7k6NQtizaOwADAC4ABHwdAMt+cRjRytmbBT4RVqFYMTGm
/jdIIYJnZ3uTo1AAOwAsAAC2KwD9////hqbXlTl39C5ZmAtOAI4+zy32vWLVnyQjw7IvQsBa6rYA
AAcAAP3//zKG/zqgb7JIwmli68xeUjUXVp+c4b/tvc55IsAI4yQAAAAAAAAA/f///8t/0cpxGNmb
BT4RVqFYMTOm/jdIIXj///8AAAAAgNo7AACAAAD9//3/AiHf9QUAAAAAFgAU9/Ykq9
...
(https://github.com/bitcoin/bitcoin/issues/34000)
From: https://github.com/bitcoin/bitcoin/pull/29675#discussion_r2585106930:
> looks like this line crashes:
```bash
$ echo 'cHNidP8BAP0+AQIAAAAGy4dxGNHKAQgFPtEAoVgyMqb+N0ghgmdne5OjULYs2sUAAAr/AP3////L
fnEY0crZmwU+EVahszExpv43SCGCZ2d7k6NQtizaOwADAC4ABHwdAMt+cRjRytmbBT4RVqFYMTGm
/jdIIYJnZ3uTo1AAOwAsAAC2KwD9////hqbXlTl39C5ZmAtOAI4+zy32vWLVnyQjw7IvQsBa6rYA
AAcAAP3//zKG/zqgb7JIwmli68xeUjUXVp+c4b/tvc55IsAI4yQAAAAAAAAA/f///8t/0cpxGNmb
BT4RVqFYMTOm/jdIIXj///8AAAAAgNo7AACAAAD9//3/AiHf9QUAAAAAFgAU9/Ykq9
...
β
fanquake closed an issue: "fuzz: crash in psbt fuzzer"
(https://github.com/bitcoin/bitcoin/issues/34000)
(https://github.com/bitcoin/bitcoin/issues/34000)
π¬ fanquake commented on issue "FUZZ=psbt in musig2, runs into uninitialized read":
(https://github.com/bitcoin/bitcoin/issues/33999#issuecomment-3606863179)
cc @achow101 @fjahr @rkrux
(https://github.com/bitcoin/bitcoin/issues/33999#issuecomment-3606863179)
cc @achow101 @fjahr @rkrux
π¬ hebasto commented on pull request "doc: Add `x86_64-w64-mingw32ucrt` triplet to `depends/README.md`":
(https://github.com/bitcoin/bitcoin/pull/33857#discussion_r2585127019)
> > developers on Ubuntu, for example, may still want to build for x86_64-w64-mingw32.
>
> Are we supporting that, if the CI is removed in #33593 (along with dropping workarounds going forward).
We won't support that once workarounds are dropped and code become incompatible with MSVCRT. By that point, I donβt anticipate any issues with providing that support.
(https://github.com/bitcoin/bitcoin/pull/33857#discussion_r2585127019)
> > developers on Ubuntu, for example, may still want to build for x86_64-w64-mingw32.
>
> Are we supporting that, if the CI is removed in #33593 (along with dropping workarounds going forward).
We won't support that once workarounds are dropped and code become incompatible with MSVCRT. By that point, I donβt anticipate any issues with providing that support.
π¬ fanquake commented on pull request "doc: Add `x86_64-w64-mingw32ucrt` triplet to `depends/README.md`":
(https://github.com/bitcoin/bitcoin/pull/33857#discussion_r2585133303)
> By that point,
Isn't that point #33593? The workarounds should be removed along with the CI being dropped, otherwise they are untested and unused.
(https://github.com/bitcoin/bitcoin/pull/33857#discussion_r2585133303)
> By that point,
Isn't that point #33593? The workarounds should be removed along with the CI being dropped, otherwise they are untested and unused.
π sedited approved a pull request: "index: remove unnecessary locator cleaning in BaseIndex::Init()"
(https://github.com/bitcoin/bitcoin/pull/32882#pullrequestreview-3535074920)
ACK facd01e6ffbbd019312f370a3807de0b95bbd745
(https://github.com/bitcoin/bitcoin/pull/32882#pullrequestreview-3535074920)
ACK facd01e6ffbbd019312f370a3807de0b95bbd745
π¬ hebasto commented on pull request "doc: Add `x86_64-w64-mingw32ucrt` triplet to `depends/README.md`":
(https://github.com/bitcoin/bitcoin/pull/33857#discussion_r2585143654)
> Isn't that point #33593? The workarounds should be removed along with the CI being dropped, otherwise they are untested and unused.
It could be in #33593 or in a follow-up PR.
(https://github.com/bitcoin/bitcoin/pull/33857#discussion_r2585143654)
> Isn't that point #33593? The workarounds should be removed along with the CI being dropped, otherwise they are untested and unused.
It could be in #33593 or in a follow-up PR.
π¬ fanquake commented on pull request "depends: Propagate native C compiler to `sqlite` package":
(https://github.com/bitcoin/bitcoin/pull/33995#issuecomment-3606913212)
Concept ACK over #33975.
(https://github.com/bitcoin/bitcoin/pull/33995#issuecomment-3606913212)
Concept ACK over #33975.
π fanquake merged a pull request: "index: remove unnecessary locator cleaning in BaseIndex::Init()"
(https://github.com/bitcoin/bitcoin/pull/32882)
(https://github.com/bitcoin/bitcoin/pull/32882)
π fanquake merged a pull request: "doc: Add `x86_64-w64-mingw32ucrt` triplet to `depends/README.md`"
(https://github.com/bitcoin/bitcoin/pull/33857)
(https://github.com/bitcoin/bitcoin/pull/33857)