Bitcoin Core Github
44 subscribers
120K links
Download Telegram
💬 apogio commented on pull request "Add coins (UTXOs) tab and makes it view-only":
(https://github.com/bitcoin-core/gui/pull/898#issuecomment-3370131703)
So, I've got 3 failed pipeline checks. I'll need to check them.
💬 maflcko commented on issue "v30rc2 createrawtransaction unable to create txns with multiple OP_RETURNs":
(https://github.com/bitcoin/bitcoin/issues/33544#issuecomment-3370140629)
With policy changes, the expectation is that one release is needed to upgrade a sufficient portion of the network, until the wallet and RPC can offer them as features. Otherwise, users may not be able to propagate such transactions, or only intermittently.
💬 maflcko commented on issue "ARM Windows build and release":
(https://github.com/bitcoin/bitcoin/issues/31388#issuecomment-3370158680)
I guess we could vendor the toolchain in guix ourselves, so re-opening for now. Though, I am not sure if we should vendor it ourselves.
💬 mikekelly commented on issue "v30rc2 createrawtransaction unable to create txns with multiple OP_RETURNs":
(https://github.com/bitcoin/bitcoin/issues/33544#issuecomment-3370161045)
@maflcko genuine question - why couldn't the policy be to accommodate policy changes in the same release but with a warning to users about potential propagation issues (and then remove it in the next release)?
💬 maflcko commented on issue "v30rc2 createrawtransaction unable to create txns with multiple OP_RETURNs":
(https://github.com/bitcoin/bitcoin/issues/33544#issuecomment-3370168020)
Yeah, it is always a bit of a case-by-case decision. However, I don't think a pull request in a reviewable state exists to implement this? So nothing can be merged into 31.x, nor 30.x anyway.
💬 BenWestgate commented on pull request "doc: update multisig tutorial to use multipath descriptors":
(https://github.com/bitcoin/bitcoin/pull/33286#discussion_r2405153857)
I updated the note with your feedback and implemented the comment nit in the test. Does this address the confusion?

> Note that previously at least two descriptors were usually used, one for external derivation paths and one for internal ones. Since https://github.com/bitcoin/bitcoin/pull/22838 this redundancy has been eliminated by a multipath descriptor with <code><0;1></code> at the [BIP-44](https://github.com/bitcoin/bips/blob/master/bip-0044.mediawiki#change) change level expanding to e
...
👍 hodlinator approved a pull request: "Clear out space on centos job"
(https://github.com/bitcoin/bitcoin/pull/33514#pullrequestreview-3303220896)
ACK 014b9f6268390b78e66e1be24813d3f069588442

Was curious if it would take less time to switch to the *stream10-minimal container*, but it seems to come with the same amount of */usr/local/lib/android* files, taking over 3min to remove regardless (https://github.com/hodlinator/bitcoin/actions/runs/18273895919/job/52021497688#step:6:249).
💬 hodlinator commented on pull request "Clear out space on centos job":
(https://github.com/bitcoin/bitcoin/pull/33514#discussion_r2405218839)
nit: Remove leftovers, unless they serve a purpose?
```suggestion
freeup-space: ${{ needs.runners.outputs.provider == 'gha' }} # Only needed on hosted GH runners (forks)
```
💬 hodlinator commented on pull request "Clear out space on centos job":
(https://github.com/bitcoin/bitcoin/pull/33514#discussion_r2405228550)
Don't see a problem with the current approach. Would just prefer:
```suggestion
if: ${{ matrix.freeup-space }}
```
💬 maflcko commented on pull request "Release: 30.0rc3 translations update":
(https://github.com/bitcoin/bitcoin/pull/33541#issuecomment-3370423916)
Just as a note: It will likely never be possible to fully please the LLM. It is still experimental and WIP, so not all issues have to be addressed.
Though, this lgtm ACK 71ee0163dedd28327993415120e864253b127f8e
💬 theStack commented on pull request "test: add functional test for `TestShell` (matching doc example)":
(https://github.com/bitcoin/bitcoin/pull/33546#issuecomment-3370425877)
Force-pushed with a fix to make the test also work for non-wallet builds. With this, all CI jobs should pass now.

> Looks like the new test times out in CI?

Hmm, seems like the test just hangs whenever the `TestShell` instance is not shut down at the end. Wrapped it in an `try: ... finally: test.shutdown()` block in order to prevent time-outs if any exception would be thrown there.
💬 maflcko commented on pull request "Clear out space on centos job":
(https://github.com/bitcoin/bitcoin/pull/33514#discussion_r2405299263)
> Don't see a problem with the current approach.

The problem is that GHA does not run in this repo, so there likely won't be any indication when this setting is needed. It can practically most likely only be removed/added in a follow-up, which increases the churn a bit.

If the overhead is too large, the step could be run in the background, albeit that would be dropping the reporting feature.

No strong opinion, though, just wanted to mention it.
💬 maflcko commented on pull request "Clear out space on centos job":
(https://github.com/bitcoin/bitcoin/pull/33514#issuecomment-3370449972)
> Was curious if it would take less time to switch to the _stream10-minimal container_,

Just for reference, the added step here runs before any of the CI logic is run, so changing the CI config should not have any effect on this step here.
💬 hodlinator commented on pull request "Clear out space on centos job":
(https://github.com/bitcoin/bitcoin/pull/33514#issuecomment-3370455740)
> > Was curious if it would take less time to switch to the _stream10-minimal container_,
>
> Just for reference, the added step here runs before any of the CI logic is run, so changing the CI config should not have any effect on this step here.

Yeah, just realized `/usr/local/lib/android` was being removed on the host OS, not inside the container.
💬 willcl-ark commented on pull request "Clear out space on centos job":
(https://github.com/bitcoin/bitcoin/pull/33514#discussion_r2405313117)
Yeah it's a fair shout.

The step in my run in OP took 4 mins, but I have seen it also take 2 minutes. Either way that's "wasted time" on the run, although this doesn't particularly matter on a fork run IMO, as they're way less time-sensitive, less likely to be queued up with a backlog (and using free compute).

When each job is takining in the order of 1-2 hours, adding 4 minutes on doesnt' feel that problematic to me.
💬 janb84 commented on pull request "doc: Add `INSTALL.md` to Linux release tarballs":
(https://github.com/bitcoin/bitcoin/pull/33451#issuecomment-3370466568)
> [Feedback](https://github.com/bitcoin/bitcoin/pull/33451#discussion_r2402371308) from @fanquake has been addressed.

I re-tested this but without `libxcb-cursor` I cannot start bitcoin-qt on Xcfe / Debian 13

```sh
./bitcoin-qt
./bitcoin-qt: error while loading shared libraries: libxcb-cursor.so.0: cannot open shared object file: No such file or directory
```
💬 rkrux commented on pull request "doc: update multisig tutorial to use multipath descriptors":
(https://github.com/bitcoin/bitcoin/pull/33286#discussion_r2405318692)
Yes, this handles it. Thank you.
👍 rkrux approved a pull request: "doc: update multisig tutorial to use multipath descriptors"
(https://github.com/bitcoin/bitcoin/pull/33286#pullrequestreview-3303358609)
crACK 2a46e94
💬 willcl-ark commented on pull request "doc: Add `INSTALL.md` to Linux release tarballs":
(https://github.com/bitcoin/bitcoin/pull/33451#issuecomment-3370478414)
@janb84 you'll need to guix build a binary from a commit post #33434 (or for v30 with #33473 checked out)