💬 Retropex commented on pull request "datacarriersize: Match more datacarrying":
(https://github.com/bitcoin/bitcoin/pull/28408#issuecomment-1845105629)
> Next move?
Standardize the new spam method.
(https://github.com/bitcoin/bitcoin/pull/28408#issuecomment-1845105629)
> Next move?
Standardize the new spam method.
💬 maflcko commented on issue "./contrib/guix/guix-build does not work on riscv64":
(https://github.com/bitcoin/bitcoin/issues/29020#issuecomment-1845106572)
The other targets seem to be affected as well. (Tested Windows as well, so far)
The guix version is 1.4, but that shouldn't matter, because of the time-machine.
(https://github.com/bitcoin/bitcoin/issues/29020#issuecomment-1845106572)
The other targets seem to be affected as well. (Tested Windows as well, so far)
The guix version is 1.4, but that shouldn't matter, because of the time-machine.
💬 dergoegge commented on pull request "wip: Split fuzz binary":
(https://github.com/bitcoin/bitcoin/pull/29010#issuecomment-1845118486)
> What does this mean? I'm seeing file sizes of 448MB (with fuzzing enabled) and 146MB (with a normal build) for test/fuzz/fuzz.
If you build this pull request you should see a binary per fuzz harness in `src/test/fuzz` (e.g. `src/test/fuzz/process_message`), as well as the usual fuzz binary `src/test/fuzz/fuzz`. If you sum up the size of the individual per harness binaries (assuming you compile with LTO) you should end up with roughly 3.6GB (maybe this depends on compiler version etc.). The
...
(https://github.com/bitcoin/bitcoin/pull/29010#issuecomment-1845118486)
> What does this mean? I'm seeing file sizes of 448MB (with fuzzing enabled) and 146MB (with a normal build) for test/fuzz/fuzz.
If you build this pull request you should see a binary per fuzz harness in `src/test/fuzz` (e.g. `src/test/fuzz/process_message`), as well as the usual fuzz binary `src/test/fuzz/fuzz`. If you sum up the size of the individual per harness binaries (assuming you compile with LTO) you should end up with roughly 3.6GB (maybe this depends on compiler version etc.). The
...
💬 maflcko commented on pull request "wip: Split fuzz binary":
(https://github.com/bitcoin/bitcoin/pull/29010#discussion_r1418771429)
Is the change in this header file needed for your approach? If there is only one fuzz target per file, and only one file is compiled, you wouldn't need to check whether it needs to be compiled, no?
(https://github.com/bitcoin/bitcoin/pull/29010#discussion_r1418771429)
Is the change in this header file needed for your approach? If there is only one fuzz target per file, and only one file is compiled, you wouldn't need to check whether it needs to be compiled, no?
💬 dergoegge commented on pull request "wip: Split fuzz binary":
(https://github.com/bitcoin/bitcoin/pull/29010#discussion_r1418785689)
Right, I think once building all the individual binaries is optional, this could be used to build just one of the individual binaries, e.g.:
```
CPPFLAGS="-DFUZZ_HARNESS=process_message" ./configure --enable-fuzz
make
```
which would produce `src/test/fuzz/fuzz` that only has the `process_message` harness. Building and linking just one harness is faster than all of them, maybe useful when reproducing testcases?
Maybe there is a better way of accomplishing this?
(https://github.com/bitcoin/bitcoin/pull/29010#discussion_r1418785689)
Right, I think once building all the individual binaries is optional, this could be used to build just one of the individual binaries, e.g.:
```
CPPFLAGS="-DFUZZ_HARNESS=process_message" ./configure --enable-fuzz
make
```
which would produce `src/test/fuzz/fuzz` that only has the `process_message` harness. Building and linking just one harness is faster than all of them, maybe useful when reproducing testcases?
Maybe there is a better way of accomplishing this?
📝 maflcko opened a pull request: "refactor: rpc: Pass CBlockIndex by reference instead of pointer "
(https://github.com/bitcoin/bitcoin/pull/29021)
Follow-up to https://github.com/bitcoin/bitcoin/pull/29003#issuecomment-1841435462
(https://github.com/bitcoin/bitcoin/pull/29021)
Follow-up to https://github.com/bitcoin/bitcoin/pull/29003#issuecomment-1841435462
💬 fanquake commented on issue "./contrib/guix/guix-build does not work on riscv64":
(https://github.com/bitcoin/bitcoin/issues/29020#issuecomment-1845149984)
> The guix version is 1.4, but that shouldn't matter, because of the time-machine.
I'm not sure we should make that assumption here. If the version of Guix (1.4), can't properly setup containers on the system (which looks like it could be the issue), and the fixes only landed after 1.4, then builds will still fail. It would be good to get a smaller reproducer of the issue. Does something like `guix shell --container --pure some_package` work?
(https://github.com/bitcoin/bitcoin/issues/29020#issuecomment-1845149984)
> The guix version is 1.4, but that shouldn't matter, because of the time-machine.
I'm not sure we should make that assumption here. If the version of Guix (1.4), can't properly setup containers on the system (which looks like it could be the issue), and the fixes only landed after 1.4, then builds will still fail. It would be good to get a smaller reproducer of the issue. Does something like `guix shell --container --pure some_package` work?
💬 maflcko commented on pull request "wip: Split fuzz binary":
(https://github.com/bitcoin/bitcoin/pull/29010#discussion_r1418793132)
> Building and linking just one harness is faster than all of them, maybe useful when reproducing testcases?
Not sure. This isn't supported for any other tests (bench, unit, ...), so maybe leave as a follow-up.
If building takes a long time, you can use more CPUs or a populated `ccache`.
Also, it could be done easier inside the makefile by just skipping over the other files, if needed?
(https://github.com/bitcoin/bitcoin/pull/29010#discussion_r1418793132)
> Building and linking just one harness is faster than all of them, maybe useful when reproducing testcases?
Not sure. This isn't supported for any other tests (bench, unit, ...), so maybe leave as a follow-up.
If building takes a long time, you can use more CPUs or a populated `ccache`.
Also, it could be done easier inside the makefile by just skipping over the other files, if needed?
💬 maflcko commented on issue "./contrib/guix/guix-build does not work on riscv64":
(https://github.com/bitcoin/bitcoin/issues/29020#issuecomment-1845156614)
```
# guix shell --container --pure hello
accepted connection from pid 3269077, user root
Backtrace:
19 (apply-smob/0 #<thunk 3f887712e0>)
In ice-9/boot-9.scm:
724:2 18 (call-with-prompt _ _ #<procedure default-prompt-handle?>)
In ice-9/eval.scm:
619:8 17 (_ #(#(#<directory (guile-user) 3f88776c80>)))
In guix/ui.scm:
2275:7 16 (run-guix . _)
2238:10 15 (run-guix-command _ . _)
In ice-9/boot-9.scm:
1752:10 14 (with-exception-handler _ _ #:unwind? _ # _)
1
...
(https://github.com/bitcoin/bitcoin/issues/29020#issuecomment-1845156614)
```
# guix shell --container --pure hello
accepted connection from pid 3269077, user root
Backtrace:
19 (apply-smob/0 #<thunk 3f887712e0>)
In ice-9/boot-9.scm:
724:2 18 (call-with-prompt _ _ #<procedure default-prompt-handle?>)
In ice-9/eval.scm:
619:8 17 (_ #(#(#<directory (guile-user) 3f88776c80>)))
In guix/ui.scm:
2275:7 16 (run-guix . _)
2238:10 15 (run-guix-command _ . _)
In ice-9/boot-9.scm:
1752:10 14 (with-exception-handler _ _ #:unwind? _ # _)
1
...
💬 fanquake commented on issue "./contrib/guix/guix-build does not work on riscv64":
(https://github.com/bitcoin/bitcoin/issues/29020#issuecomment-1845157887)
Ok. So the issue is a bug in Guix. How many weeks will it take to run a `guix pull`?
(https://github.com/bitcoin/bitcoin/issues/29020#issuecomment-1845157887)
Ok. So the issue is a bug in Guix. How many weeks will it take to run a `guix pull`?
💬 maflcko commented on issue "./contrib/guix/guix-build does not work on riscv64":
(https://github.com/bitcoin/bitcoin/issues/29020#issuecomment-1845164211)
Happy to try it, but if that works, then there is a bug in the time-machine, which seems unlikely.
```
# guix time-machine --commit=77386bdbfe6b0c649c05ab37f08051d1ab3e5074 -- shell --container --pure bash
accepted connection from pid 3269629, user root
accepted connection from pid 3269629, user root
Backtrace:
18 (primitive-load "/root/.cache/guix/inferiors/4wrulds2lx?")
In guix/ui.scm:
2324:7 17 (run-guix . _)
2287:10 16 (run-guix-command _ . _)
In ice-9/boot-9.scm:
...
(https://github.com/bitcoin/bitcoin/issues/29020#issuecomment-1845164211)
Happy to try it, but if that works, then there is a bug in the time-machine, which seems unlikely.
```
# guix time-machine --commit=77386bdbfe6b0c649c05ab37f08051d1ab3e5074 -- shell --container --pure bash
accepted connection from pid 3269629, user root
accepted connection from pid 3269629, user root
Backtrace:
18 (primitive-load "/root/.cache/guix/inferiors/4wrulds2lx?")
In guix/ui.scm:
2324:7 17 (run-guix . _)
2287:10 16 (run-guix-command _ . _)
In ice-9/boot-9.scm:
...
💬 Sjors commented on pull request "wallet: rpc to add automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/25907#issuecomment-1845165287)
You could also test `createwalletdescriptor` by creating a descriptor wallet using a pre-taproot version. You can check the xpub in the resulting descriptor using #22341. That way all the key rotation stuff and the new records can be done in another PR.
(https://github.com/bitcoin/bitcoin/pull/25907#issuecomment-1845165287)
You could also test `createwalletdescriptor` by creating a descriptor wallet using a pre-taproot version. You can check the xpub in the resulting descriptor using #22341. That way all the key rotation stuff and the new records can be done in another PR.
💬 fanquake commented on issue "./contrib/guix/guix-build does not work on riscv64":
(https://github.com/bitcoin/bitcoin/issues/29020#issuecomment-1845167698)
> then there is a bug in the time-machine, which seems unlikely.
I don't follow. This is failing in the same way, because time-machine is trying to use a container. So it is the same issue.
(https://github.com/bitcoin/bitcoin/issues/29020#issuecomment-1845167698)
> then there is a bug in the time-machine, which seems unlikely.
I don't follow. This is failing in the same way, because time-machine is trying to use a container. So it is the same issue.
📝 kashifs opened a pull request: "Make bitcoin-tx replaceable value optional, #fixes 28638"
(https://github.com/bitcoin/bitcoin/pull/29022)
<!--
*** 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/29022)
<!--
*** 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
...
💬 maflcko commented on issue "./contrib/guix/guix-build does not work on riscv64":
(https://github.com/bitcoin/bitcoin/issues/29020#issuecomment-1845171572)
I mean that `guix pull` likely won't fix this issue, because if this issue was fixed in a later version of guix, I could simply use the time-machine to run that version of guix. However, that didn't work.
(https://github.com/bitcoin/bitcoin/issues/29020#issuecomment-1845171572)
I mean that `guix pull` likely won't fix this issue, because if this issue was fixed in a later version of guix, I could simply use the time-machine to run that version of guix. However, that didn't work.
💬 fanquake commented on issue "./contrib/guix/guix-build does not work on riscv64":
(https://github.com/bitcoin/bitcoin/issues/29020#issuecomment-1845172844)
> I could simply use the time-machine to run that version of guix
You can't though. Because the guix binary itself, that is trying to run the container, for the time-machine, doesn't have the bug fixes from the future.
(https://github.com/bitcoin/bitcoin/issues/29020#issuecomment-1845172844)
> I could simply use the time-machine to run that version of guix
You can't though. Because the guix binary itself, that is trying to run the container, for the time-machine, doesn't have the bug fixes from the future.
💬 Sjors commented on pull request "wallet: Have the wallet store the key for automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/26728#issuecomment-1845173418)
As much as I don't like having to go through half the PR again, I do think this is a nice simplification.
(https://github.com/bitcoin/bitcoin/pull/26728#issuecomment-1845173418)
As much as I don't like having to go through half the PR again, I do think this is a nice simplification.
💬 maflcko commented on issue "./contrib/guix/guix-build does not work on riscv64":
(https://github.com/bitcoin/bitcoin/issues/29020#issuecomment-1845190697)
Are you sure, because if new guix features are available in the the time-machine, then bug fixes should be as well?
Looking at the code, it seems more likely that someone forgot to add support for riscv64 somewhere, because the code looks mostly unchanged between 1.4 and master.
https://github.com/guix-mirror/guix/blame/v1.4.0/guix/build/syscalls.scm#L1109
https://github.com/guix-mirror/guix/blame/77386bdbfe6b0c649c05ab37f08051d1ab3e5074/guix/build/syscalls.scm#L1114
(https://github.com/bitcoin/bitcoin/issues/29020#issuecomment-1845190697)
Are you sure, because if new guix features are available in the the time-machine, then bug fixes should be as well?
Looking at the code, it seems more likely that someone forgot to add support for riscv64 somewhere, because the code looks mostly unchanged between 1.4 and master.
https://github.com/guix-mirror/guix/blame/v1.4.0/guix/build/syscalls.scm#L1109
https://github.com/guix-mirror/guix/blame/77386bdbfe6b0c649c05ab37f08051d1ab3e5074/guix/build/syscalls.scm#L1114
💬 fanquake commented on issue "./contrib/guix/guix-build does not work on riscv64":
(https://github.com/bitcoin/bitcoin/issues/29020#issuecomment-1845195847)
> Are you sure, because if new guix features are available in the the time-machine, then bug fixes should be as well?
Yea, new Guix features could be compiled/available inside the time-machine, but if you can't even run a time-machine, because your guix 1.4 binary can't launch a container, you are still stuck.
(https://github.com/bitcoin/bitcoin/issues/29020#issuecomment-1845195847)
> Are you sure, because if new guix features are available in the the time-machine, then bug fixes should be as well?
Yea, new Guix features could be compiled/available inside the time-machine, but if you can't even run a time-machine, because your guix 1.4 binary can't launch a container, you are still stuck.
💬 Sjors commented on pull request "wallet: Have the wallet store the key for automatically generated descriptors":
(https://github.com/bitcoin/bitcoin/pull/26728#discussion_r1418839247)
You could create a pre-v22 wallet, upgrade to master (which won't add the `tr()` descriptor) and then continue the test.
Although I don't think we care that deeply about downgrading that far, testing a wallet without a `tr()` descriptor is useful - for followup PR's that enable adding that desciptor.
(https://github.com/bitcoin/bitcoin/pull/26728#discussion_r1418839247)
You could create a pre-v22 wallet, upgrade to master (which won't add the `tr()` descriptor) and then continue the test.
Although I don't think we care that deeply about downgrading that far, testing a wallet without a `tr()` descriptor is useful - for followup PR's that enable adding that desciptor.