💬 maflcko commented on pull request "rpc: introduce getversion RPC":
(https://github.com/bitcoin/bitcoin/pull/30112#issuecomment-2179919053)
lgtm ACK d83c12d635263fda74392eff372792e983749adb

💬 vasild commented on pull request "util: check for errors after close and read in AutoFile":
(https://github.com/bitcoin/bitcoin/pull/29307#discussion_r1647059893)
Well, it is pretty much obvious that checking for an error after read is either:

* a noop, if the read could have never failed because it managed to read as many bytes as requested or
* a bug fix where a read error was ignored before the fix

so this is a safe change (it does not break anything), possibly fixing a bug. Given that you keep pushing against this I will drop this commit in the hopes to move this PR forward.

💬 maflcko commented on pull request "util: check for errors after close and read in AutoFile":
(https://github.com/bitcoin/bitcoin/pull/29307#discussion_r1647069077)
No, I don't understand why a "read error was ignored before the fix" and why "this is a safe change (it does not break anything)". For example, if this was hit on a filesystem that stored a block file, which was fully and successfully read, but then return an error (for an unknown reason), this would mean that for a user where the program previously worked fine without any issues, is now unusable (for an unknown reason).

💬 maflcko commented on issue "automake script error building 32 bit depends libevent-2.1.12":
(https://github.com/bitcoin/bitcoin/issues/30311#issuecomment-2180032055)
As well as: https://github.com/bitcoin/bitcoin/blob/27.x/doc/build-unix.md#ubuntu--debian

💬 Sjors commented on pull request "Support self-hosted Cirrus workers on forks":
(https://github.com/bitcoin/bitcoin/pull/29274#discussion_r1647137983)
I'm fine with just skipping that action on the stratum v2 based fork.

It's probably not too much hassle if there's clear instructions on how to do that on e.g. an AMD based Ubuntu machine.

The other way around doesn't work for me, because I don't have a native ARM machine (mac or otherwise). I'll eventually have one, a laptop. I might then use that for just the ARM task, not the rest of CI.

> moving it to GHA

I don't know how common ARM is for servers these days? I'm assuming it's mo
...

💬 Sjors commented on issue "ci: Move more tasks to GHA?":
(https://github.com/bitcoin/bitcoin/issues/30304#issuecomment-2180068529)
> This is one of the reasons why I personally don't like GHA: It is a closed, confusing, and brittle ecosystem. The only benefit is that it is free (for now).

I'm a bit hesitant as well. As I mentioned in https://github.com/bitcoin/bitcoin/pull/29274#discussion_r1647137983 the only practical need I have currently is to run the native ARM job on Github CI.

However, I'm fine with either skipping it, or following some (clear) instructions to run it on my AMD desktop with some virtualisation (
...

💬 ismaelsadeeq commented on pull request "Fee Estimation: change `estimatesmartfee` default mode to `economical`":
(https://github.com/bitcoin/bitcoin/pull/30275#discussion_r1647150035)
I Added it to the PR description

💬 ismaelsadeeq commented on pull request "Wallet: Add `max_tx_weight` to transaction funding options (take 2)":
(https://github.com/bitcoin/bitcoin/pull/29523#issuecomment-2180079920)
thanks for your review @furszy will force push to address this comments shortly.

💬 Sjors commented on pull request "net: Replace libnatpmp with built-in PCP+NATPMP implementation":
(https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1647158425)
Good point, maybe just add a comment about this. FreeBSD 13 won't be EOL for another two years, so someone else might wonder.

💬 vasild commented on pull request "util: check for errors after close and read in AutoFile":
(https://github.com/bitcoin/bitcoin/pull/29307#discussion_r1647164965)
I just found a similar code to this proposed change:

https://github.com/bitcoin/bitcoin/blob/2d21060af831fa8f8f1791b4464961d5f28a88cb/src/node/utxo_snapshot.cpp#L74-L80

After reading from a file and getting everything that is needed from it, the code checks if there is an extra data and if that read fails it reports an error.

💬 maflcko commented on pull request "Support self-hosted Cirrus workers on forks":
(https://github.com/bitcoin/bitcoin/pull/29274#discussion_r1647165078)
> I don't know how common ARM is for servers these days? I'm assuming it's mostly used by Apple, and for very low performance devices like Raspberry Pi's.

ARM is common in servers and all major cloud providers offer it. GHA does not offer it for Linux, only macOS M1, which is why I was wondering if it is possible to run ARM Linux containers on macOS M1?

> It's probably not too much hassle if there's clear instructions on how to do that on e.g. an AMD based Ubuntu machine.

It depends on
...

💬 Sjors commented on pull request "Stratum v2 Noise Protocol":
(https://github.com/bitcoin/bitcoin/pull/29346#issuecomment-2180091233)
I'm still hopeful sv1 translators won't be needed at all soon(tm). That certainly seems too complicated to implement in Bitcoin Core.

For the same reason I don't think combining JDC and Translator is a good long term solution, though in the short run it could make the SRI side of things easier to configure.

See also the conceptual discussion in https://github.com/stratum-mining/sv2-spec/discussions/85

💬 Sjors commented on pull request "Support self-hosted Cirrus workers on forks":
(https://github.com/bitcoin/bitcoin/pull/29274#discussion_r1647173189)
After `podman run` is there some VM running that I have to login to, install the Cirrus job runner? Or is there some change needed to `00_setup_env_arm.sh` to make it use this?

💬 maflcko commented on issue "ci: Move more tasks to GHA?":
(https://github.com/bitcoin/bitcoin/issues/30304#issuecomment-2180100968)
> The other jobs run fine on my Ubuntu machine(s) with not too much configuration.

Sure, but for others it may be too much hassle? See https://github.com/bitcoin-inquisition/bitcoin/pull/32#issue-1874824335

💬 maflcko commented on pull request "Support self-hosted Cirrus workers on forks":
(https://github.com/bitcoin/bitcoin/pull/29274#discussion_r1647183350)
You don't need to log in to anything, and everything should work out of the box without code modifications. I can try to test, if you provide more details about your exact system, and whether you intend to use podman-docker or docker.

💬 Sjors commented on issue "ci: Move more tasks to GHA?":
(https://github.com/bitcoin/bitcoin/issues/30304#issuecomment-2180115136)
They didn't try self-hosting _or_ paying. It doesn't seem like a good strategy to constantly flock to whichever company offers free resources. It would be nice if we can make it more flexible in an easy way.

E.g. someone maintaining a fork could upload a yaml file somewhere that specifies which jobs should be run by which cloud provider / self host, and which should be skipped.

💬 maflcko commented on pull request "util: check for errors after close and read in AutoFile":
(https://github.com/bitcoin/bitcoin/pull/29307#discussion_r1647191325)
The code you linked to is primarily checking for unexpected trailing data and only printing a warning. It is not making the program possibly unusable.

💬 fanquake commented on pull request "QA: Expect PACKAGE_NAME rather than constant "Bitcoin Core"":
(https://github.com/bitcoin/bitcoin/pull/30308#issuecomment-2180131521)
> not sure if you want to update here aswell in this PR

@luke-jr can you update this PR to include the additional changes.

💬 fanquake commented on pull request "depends: bump miniupnpc to 2.2.8":
(https://github.com/bitcoin/bitcoin/pull/30301#issuecomment-2180137005)
> Marked as draft until https://github.com/bitcoin/bitcoin/pull/30283 is merged.

Can rebase / undraft this now.

✅ fanquake closed an issue: "fuzz: wallet_bdb_parser: implicit-signed-integer-truncation wallet/migrate.cpp:554:35 "
(https://github.com/bitcoin/bitcoin/issues/30247)