🤔 glozow reviewed a pull request: "test: create assert_less_than util"
(https://github.com/bitcoin/bitcoin/pull/30019#pullrequestreview-2035382457)
Why not just use `assert_greater_than_or_equal`?

💬 paplorinc commented on pull request "refactor, fuzz: Make 64-bit shift explicit":
(https://github.com/bitcoin/bitcoin/pull/30017#discussion_r1587407613)
was the `1U` deliberately kept here?

💬 maflcko commented on pull request "refactor: Avoid unused-variable warning in init.cpp":
(https://github.com/bitcoin/bitcoin/pull/29968#issuecomment-2090190099)
> Avoids the worry about future compilers, and (void)blah which isn't explanatory

I think `(void)blah;` is well understood to mean this (to both code readers and compilers), but `[[maybe_unused]]` seems fine as well. Switched to that.

📝 Cryptocurrencei opened a pull request: "Rename CONTRIBUTING.md to CONTRIBUTING.cryptocurrency.md"
(https://github.com/bitcoin/bitcoin/pull/30021)
<!--
*** 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
...

💬 glozow commented on pull request "p2p: index TxOrphanage by wtxid, allow entries with same txid":
(https://github.com/bitcoin/bitcoin/pull/30000#discussion_r1587460520)
> It's not immediately obvious to me why it is safe to just convert any gtxid into a Wtxid here

The argument I'm making in this PR is that in all the places we are checking by txid, we shouldn't be doing that, because we're missing same-txid-different-witness cases. But yeah totally fair to say it's difficult to review it together, so I'll split the first commit to make it more explicit where the change is happening and why that's ok.

> I think it could be a better approach to update Alre
...

💬 dominicusadinfinitum commented on issue "Testsuite for Bitcoin Core 27.0.0 - FAIL: qt/test/test_bitcoin-qt":
(https://github.com/bitcoin/bitcoin/issues/30020#issuecomment-2090241517)
> A few other thoughts:
>
> 1. Have you installed all the [GUI dependencies](https://github.com/bitcoin/bitcoin/blob/master/doc/build-unix.md#linux-distribution-specific-instructions)? (preumably you must have, and I see "Using QtTest library 5.15.8, Qt 5.15.8", but just thought I'd double check)
> 2. Does this happen if you configure with debug mode enabled: `./configure --enable-debug`? If so, the output may also be more useful to us to find what is failing rather than guessing from "No sy
...

✅ fanquake closed a pull request: "Rename CONTRIBUTING.md to CONTRIBUTING.cryptocurrency.md"
(https://github.com/bitcoin/bitcoin/pull/30021)

📝 fanquake locked a pull request: "Rename CONTRIBUTING.md to CONTRIBUTING.cryptocurrency.md"
(https://github.com/bitcoin/bitcoin/pull/30021)
<!--
*** 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
...

💬 dominicusadinfinitum commented on issue "Testsuite for Bitcoin Core 27.0.0 - FAIL: qt/test/test_bitcoin-qt":
(https://github.com/bitcoin/bitcoin/issues/30020#issuecomment-2090247040)
> @dominicusadinfinitum
>
> What are the first few lines (before "Script verification uses...") in the `debug.log` file when you run `bitcoin-qt -regtest`?

I'm new to this. Trying to figure out how to do that. Keep you updated.

💬 dominicusadinfinitum commented on issue "Testsuite for Bitcoin Core 27.0.0 - FAIL: qt/test/test_bitcoin-qt":
(https://github.com/bitcoin/bitcoin/issues/30020#issuecomment-2090271815)
> > A few other thoughts:
> >
> > 1. Have you installed all the [GUI dependencies](https://github.com/bitcoin/bitcoin/blob/master/doc/build-unix.md#linux-distribution-specific-instructions)? (preumably you must have, and I see "Using QtTest library 5.15.8, Qt 5.15.8", but just thought I'd double check)
> > 2. Does this happen if you configure with debug mode enabled: `./configure --enable-debug`? If so, the output may also be more useful to us to find what is failing rather than guessing fro
...

💬 hebasto commented on pull request "refactor, fuzz: Make 64-bit shift explicit":
(https://github.com/bitcoin/bitcoin/pull/30017#discussion_r1587487300)
It causes no issues?

📝 fanquake opened a pull request: "releases: use LLVM 18 for macOS"
(https://github.com/bitcoin/bitcoin/pull/30022)
Bumps the Guix time-machine to include a bump to LLVM 18.1.4: https://git.savannah.gnu.org/cgit/guix.git/commit/?id=837016fe33e11c6252ec24c423d1b6ae0cd7efd3. Can split this out given it effects all releases.

Needs another patch to Qt. It's internal libpng build is broken with the newer Clang, due to:
> A new Clang extension (see [here](https://releases.llvm.org/18.1.0/tools/clang/docs/ReleaseNotes.html?1=#target-os-detail)) is enabled for Darwin (Apple platform) targets. Clang now defines TA
...

💬 alfonsoromanz commented on pull request "test: Assumeutxo: import snapshot in a node with a divergent chain":
(https://github.com/bitcoin/bitcoin/pull/29996#discussion_r1587494035)
Actually `SNAPSHOT_BASE_HEIGHT` is 299, and here I am trying to make sure that the snapshot was deleted and therefore the node is now using it's previous chain which has less work. I changed it to `assert node.getblockcount() < SNAPSHOT_BASE_HEIGHT` for more clarity

💬 alfonsoromanz commented on pull request "test: Assumeutxo: import snapshot in a node with a divergent chain":
(https://github.com/bitcoin/bitcoin/pull/29996#discussion_r1587496652)
Yes, good catch. This line was removed since the iteration is no longer needed based on your other comment below.

💬 alfonsoromanz commented on pull request "test: Assumeutxo: import snapshot in a node with a divergent chain":
(https://github.com/bitcoin/bitcoin/pull/29996#discussion_r1587497230)
That's a good one. Thanks. I Fixed it.

💬 paplorinc commented on pull request "refactor, fuzz: Make 64-bit shift explicit":
(https://github.com/bitcoin/bitcoin/pull/30017#discussion_r1587506138)
I don't have MSVC to test, just seemed suspicious. If it was deliberate, please resolve this comment.

💬 hebasto commented on pull request "refactor, fuzz: Make 64-bit shift explicit":
(https://github.com/bitcoin/bitcoin/pull/30017#discussion_r1587527245)
> I don't have MSVC to test, just seemed suspicious.

Do you mind elaborating your concerns?

> If it was deliberate, please resolve this comment.

The MSVC warning C4334 is caused by the type of the left-hand side operand in the `<<` operator. This PR is focused on resolving only this issue. I can't see reasons to introduce unrelated code modifications.

✅ laanwj closed an issue: "Objdump can't parse our Linux debug information"
(https://github.com/bitcoin/bitcoin/issues/30016)

💬 laanwj commented on issue "Objdump can't parse our Linux debug information":
(https://github.com/bitcoin/bitcoin/issues/30016#issuecomment-2090370225)
This was already solved upstream!

Diff: https://sourceware.org/git/?p=binutils-gdb.git;a=blobdiff;f=binutils/objdump.c;h=3d70df470f290f7e4dc46cf80bc075f3015e6c14;hp=5acaa54929dbd4bbe37d7a2195ca698bfca603d0;hb=635d05b88f4823f46ef1ddbb3d438db16c0f6e71;hpb=5ce0e02478cc79a260c7e29822450284a32b9b12

i expect this will land in binutils 2.43. No need to keep this issue open.

💬 dergoegge commented on pull request "releases: use LLVM 18 for macOS":
(https://github.com/bitcoin/bitcoin/pull/30022#discussion_r1587550128)
this download link works but ubuntu-18?