💬 marcofleon commented on issue "build: clang-16 build broken on Debian Bookworm":
(https://github.com/bitcoin/bitcoin/issues/33279#issuecomment-3245714093)
Just got the same error with Clang 19. Installing the default Capn Proto packages (0.9.2) won't work it seems. I upgraded to 1.1.0 by building capnp from source and bitcoin builds now. It even builds an extra 1%...
<img width="1459" height="324" alt="Image" src="https://github.com/user-attachments/assets/19cca262-77e1-4984-ad19-ceb84c0c573d" />
(https://github.com/bitcoin/bitcoin/issues/33279#issuecomment-3245714093)
Just got the same error with Clang 19. Installing the default Capn Proto packages (0.9.2) won't work it seems. I upgraded to 1.1.0 by building capnp from source and bitcoin builds now. It even builds an extra 1%...
<img width="1459" height="324" alt="Image" src="https://github.com/user-attachments/assets/19cca262-77e1-4984-ad19-ceb84c0c573d" />
💬 m3dwards commented on pull request "ci: Migrate CI to hosted Cirrus Runners":
(https://github.com/bitcoin/bitcoin/pull/32989#issuecomment-3245714292)
ACK 3c5da69a232ba1cfb935012aa53e57002efe0d77
(https://github.com/bitcoin/bitcoin/pull/32989#issuecomment-3245714292)
ACK 3c5da69a232ba1cfb935012aa53e57002efe0d77
💬 ryanofsky commented on pull request "clang-tidy: disable clang-analyzer UndefinedBinaryOperatorResult warning":
(https://github.com/bitcoin/bitcoin/pull/33281#issuecomment-3245742642)
> Can it be localized into the `src/ipc/capnp/.clang-tidy` file?
That seems like a good idea. But even the current version of this PR (c157a1bc8388fecec85f61286ae928721aa8d78a) doesn't seem to be working right now. I wonder if it is because the files are generated files not inside the source directory? I'm not sure what change might make sense to fix this.
Current tidy job output is:
- https://cirrus-ci.com/task/5720235140972544
- https://api.cirrus-ci.com/v1/task/5720235140972544/logs
...
(https://github.com/bitcoin/bitcoin/pull/33281#issuecomment-3245742642)
> Can it be localized into the `src/ipc/capnp/.clang-tidy` file?
That seems like a good idea. But even the current version of this PR (c157a1bc8388fecec85f61286ae928721aa8d78a) doesn't seem to be working right now. I wonder if it is because the files are generated files not inside the source directory? I'm not sure what change might make sense to fix this.
Current tidy job output is:
- https://cirrus-ci.com/task/5720235140972544
- https://api.cirrus-ci.com/v1/task/5720235140972544/logs
...
📝 ryanofsky converted_to_draft a pull request: "clang-tidy: disable clang-analyzer UndefinedBinaryOperatorResult warning"
(https://github.com/bitcoin/bitcoin/pull/33281)
Disable `clang-analyzer-core.UndefinedBinaryOperatorResult` warning because it produces a false positive in a Cap'n Proto header:
```c++
/usr/include/kj/async-inl.h:609:37: warning: The left operand of '+' is a garbage value [clang-analyzer-core.UndefinedBinaryOperatorResult]
609 | return *(void**)(*(char**)obj + voff);
```
This was reported by fanquake in https://github.com/bitcoin/bitcoin/issues/33256 and was previously addressed in https://github.com/bitcoin-core/libmultiproc
...
(https://github.com/bitcoin/bitcoin/pull/33281)
Disable `clang-analyzer-core.UndefinedBinaryOperatorResult` warning because it produces a false positive in a Cap'n Proto header:
```c++
/usr/include/kj/async-inl.h:609:37: warning: The left operand of '+' is a garbage value [clang-analyzer-core.UndefinedBinaryOperatorResult]
609 | return *(void**)(*(char**)obj + voff);
```
This was reported by fanquake in https://github.com/bitcoin/bitcoin/issues/33256 and was previously addressed in https://github.com/bitcoin-core/libmultiproc
...
💬 ryanofsky commented on pull request "clang-tidy: disable clang-analyzer UndefinedBinaryOperatorResult warning":
(https://github.com/bitcoin/bitcoin/pull/33281#issuecomment-3245745115)
Marking as draft because the change does not seem to have any effect right now.
(https://github.com/bitcoin/bitcoin/pull/33281#issuecomment-3245745115)
Marking as draft because the change does not seem to have any effect right now.
💬 maflcko commented on pull request "ci: Migrate CI to hosted Cirrus Runners":
(https://github.com/bitcoin/bitcoin/pull/32989#discussion_r2316389311)
> It's not like a "normal" lint that checks code style and therefore often fails.
It is, and it fails occasionally: https://github.com/bitcoin/bitcoin/pulls?q=is%3Apr+%22task+lint%3A+https%3A%2F%2Fgithub.com%2Fbitcoin%2Fbitcoin%2Fruns%2F%22+is%3Aclosed
No opinion on the dep graph. The benefit of putting it in `needs` is that the other tasks are skipped (as they are possibly not needed). The downside of putting it in `needs` is that the other tasks are skipped (as the failure is intended/ig
...
(https://github.com/bitcoin/bitcoin/pull/32989#discussion_r2316389311)
> It's not like a "normal" lint that checks code style and therefore often fails.
It is, and it fails occasionally: https://github.com/bitcoin/bitcoin/pulls?q=is%3Apr+%22task+lint%3A+https%3A%2F%2Fgithub.com%2Fbitcoin%2Fbitcoin%2Fruns%2F%22+is%3Aclosed
No opinion on the dep graph. The benefit of putting it in `needs` is that the other tasks are skipped (as they are possibly not needed). The downside of putting it in `needs` is that the other tasks are skipped (as the failure is intended/ig
...
💬 maflcko commented on pull request "Release: 30.0 translations update":
(https://github.com/bitcoin/bitcoin/pull/33275#issuecomment-3245791098)
> LLM seems unable to consider enough context here.
Is there some additional context that translators have? If yes, it could make sense to somehow pass it to the llm. Other than that, my recommendation would be to use extra comments, like here:
```
<source>Inbound</source>
<extracomment>An inbound connection from a peer. An inbound connection is a connection initiated by a peer.</extracomment>
<translation type="unfinished">Вхідний</translation>
```
> The
...
(https://github.com/bitcoin/bitcoin/pull/33275#issuecomment-3245791098)
> LLM seems unable to consider enough context here.
Is there some additional context that translators have? If yes, it could make sense to somehow pass it to the llm. Other than that, my recommendation would be to use extra comments, like here:
```
<source>Inbound</source>
<extracomment>An inbound connection from a peer. An inbound connection is a connection initiated by a peer.</extracomment>
<translation type="unfinished">Вхідний</translation>
```
> The
...
💬 Sjors commented on pull request "Add functional test for IPC interface":
(https://github.com/bitcoin/bitcoin/pull/33201#issuecomment-3245811520)
ACK ebe87082cc482f0a0001e748e184b4e0db6ba996
I think you need to add `pycapnp` to the macOS native job so `interface_ipc.py` isn't skipped there. I'm trying that now in https://github.com/Sjors/bitcoin/pull/103.
(https://github.com/bitcoin/bitcoin/pull/33201#issuecomment-3245811520)
ACK ebe87082cc482f0a0001e748e184b4e0db6ba996
I think you need to add `pycapnp` to the macOS native job so `interface_ipc.py` isn't skipped there. I'm trying that now in https://github.com/Sjors/bitcoin/pull/103.
💬 mzumsande commented on issue "-loadblock doesn't work":
(https://github.com/bitcoin/bitcoin/issues/33280#issuecomment-3245814664)
are the two datadirs using the same xor.dat or have they disabled (see `-blocksxor`)?
(https://github.com/bitcoin/bitcoin/issues/33280#issuecomment-3245814664)
are the two datadirs using the same xor.dat or have they disabled (see `-blocksxor`)?
💬 fanquake commented on issue "cmake: Errors not actually errors?":
(https://github.com/bitcoin/bitcoin/issues/33153#issuecomment-3245855785)
Put this on v30.0, because we shouldn't ship a build system that produces errors, and then just ignores them.
(https://github.com/bitcoin/bitcoin/issues/33153#issuecomment-3245855785)
Put this on v30.0, because we shouldn't ship a build system that produces errors, and then just ignores them.
💬 jesterhodl commented on pull request "Release: 30.0 translations update":
(https://github.com/bitcoin/bitcoin/pull/33275#issuecomment-3245894816)
Apologies, but is WalletDescriptor an internal name or is it a typo and should be Wallet Descriptor?
> "Cannot add WalletDescriptor to a non-descriptor wallet"
(https://github.com/bitcoin/bitcoin/pull/33275#issuecomment-3245894816)
Apologies, but is WalletDescriptor an internal name or is it a typo and should be Wallet Descriptor?
> "Cannot add WalletDescriptor to a non-descriptor wallet"
💬 jesterhodl commented on pull request "Release: 30.0 translations update":
(https://github.com/bitcoin/bitcoin/pull/33275#issuecomment-3245904340)
What is a "rescan reserver"?
> "Failed to acquire rescan reserver during wallet initialization"
(https://github.com/bitcoin/bitcoin/pull/33275#issuecomment-3245904340)
What is a "rescan reserver"?
> "Failed to acquire rescan reserver during wallet initialization"
💬 hebasto commented on pull request "Release: 30.0 translations update":
(https://github.com/bitcoin/bitcoin/pull/33275#issuecomment-3245949200)
> Apologies, but is WalletDescriptor an internal name or is it a typo and should be Wallet Descriptor?
>
> > "Cannot add WalletDescriptor to a non-descriptor wallet"
An internal name.
(https://github.com/bitcoin/bitcoin/pull/33275#issuecomment-3245949200)
> Apologies, but is WalletDescriptor an internal name or is it a typo and should be Wallet Descriptor?
>
> > "Cannot add WalletDescriptor to a non-descriptor wallet"
An internal name.
💬 hebasto commented on pull request "Release: 30.0 translations update":
(https://github.com/bitcoin/bitcoin/pull/33275#issuecomment-3245967004)
> What is a "rescan reserver"?
>
> > "Failed to acquire rescan reserver during wallet initialization"
I commented this string on Transifex, but for some reason my explanation has gone.
Here it is now:
> It’s a special internal object that holds (or “reserves”) the wallet’s “rescan” state.
>
> While the wallet is rescanning, no unrelated access should be allowed. The “rescan reserver” makes sure of that.
(https://github.com/bitcoin/bitcoin/pull/33275#issuecomment-3245967004)
> What is a "rescan reserver"?
>
> > "Failed to acquire rescan reserver during wallet initialization"
I commented this string on Transifex, but for some reason my explanation has gone.
Here it is now:
> It’s a special internal object that holds (or “reserves”) the wallet’s “rescan” state.
>
> While the wallet is rescanning, no unrelated access should be allowed. The “rescan reserver” makes sure of that.
💬 maflcko commented on pull request "Release: 30.0 translations update":
(https://github.com/bitcoin/bitcoin/pull/33275#issuecomment-3246003431)
> > Apologies, but is WalletDescriptor an internal name or is it a typo and should be Wallet Descriptor?
> > > "Cannot add WalletDescriptor to a non-descriptor wallet"
>
> An internal name.
There are more instances of this. I'd say, generally it is best to avoid internal names and instead use the normal/general word. Here, it could be "Cannot add descriptor to a non-descriptor wallet", or "Cannot add output script descriptor to a non-descriptor wallet"?
(https://github.com/bitcoin/bitcoin/pull/33275#issuecomment-3246003431)
> > Apologies, but is WalletDescriptor an internal name or is it a typo and should be Wallet Descriptor?
> > > "Cannot add WalletDescriptor to a non-descriptor wallet"
>
> An internal name.
There are more instances of this. I'd say, generally it is best to avoid internal names and instead use the normal/general word. Here, it could be "Cannot add descriptor to a non-descriptor wallet", or "Cannot add output script descriptor to a non-descriptor wallet"?
💬 hebasto commented on pull request "Release: 30.0 translations update":
(https://github.com/bitcoin/bitcoin/pull/33275#issuecomment-3246010706)
> > > Apologies, but is WalletDescriptor an internal name or is it a typo and should be Wallet Descriptor?
> > > > "Cannot add WalletDescriptor to a non-descriptor wallet"
> >
> >
> > An internal name.
>
> There are more instances of this. I'd say, generally it is best to avoid internal names and instead use the normal/general word. Here, it could be "Cannot add descriptor to a non-descriptor wallet", or "Cannot add output script descriptor to a non-descriptor wallet"?
Yes, the origi
...
(https://github.com/bitcoin/bitcoin/pull/33275#issuecomment-3246010706)
> > > Apologies, but is WalletDescriptor an internal name or is it a typo and should be Wallet Descriptor?
> > > > "Cannot add WalletDescriptor to a non-descriptor wallet"
> >
> >
> > An internal name.
>
> There are more instances of this. I'd say, generally it is best to avoid internal names and instead use the normal/general word. Here, it could be "Cannot add descriptor to a non-descriptor wallet", or "Cannot add output script descriptor to a non-descriptor wallet"?
Yes, the origi
...
💬 Sjors commented on pull request "Add functional test for IPC interface":
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2316615719)
In e043a279da6ea901b5f6d6690f96ff0d419faf13 _ci: enable IPC tests in CI_: `pip3 install` did the trick for me, see https://github.com/Sjors/bitcoin/actions/runs/17409223017/job/49421870136?pr=103#step:8:3514
(https://github.com/bitcoin/bitcoin/pull/33201#discussion_r2316615719)
In e043a279da6ea901b5f6d6690f96ff0d419faf13 _ci: enable IPC tests in CI_: `pip3 install` did the trick for me, see https://github.com/Sjors/bitcoin/actions/runs/17409223017/job/49421870136?pr=103#step:8:3514
🤔 w0xlt reviewed a pull request: "kernel: chainparams & headersync updates for 30.0"
(https://github.com/bitcoin/bitcoin/pull/33274#pullrequestreview-3177418705)
ACK https://github.com/bitcoin/bitcoin/pull/33274/commits/755152ac819a23acf2f9e70316134d74a04d589b
(https://github.com/bitcoin/bitcoin/pull/33274#pullrequestreview-3177418705)
ACK https://github.com/bitcoin/bitcoin/pull/33274/commits/755152ac819a23acf2f9e70316134d74a04d589b
💬 l0rinc commented on pull request "rpc: followups for min fee changes":
(https://github.com/bitcoin/bitcoin/pull/33189#issuecomment-3246155677)
My intention wasn't to hurt, thanks for clarifying the title.
(https://github.com/bitcoin/bitcoin/pull/33189#issuecomment-3246155677)
My intention wasn't to hurt, thanks for clarifying the title.
💬 l0rinc commented on pull request "Release: 30.0 translations update":
(https://github.com/bitcoin/bitcoin/pull/33275#issuecomment-3246189935)
> Is there some additional context that translators have?
We could probably get more accurate translations by creating a bot that goes through each translation (<2000 values I think, we could batch them by files) fetches the source code context for the given translations and gives 3-4 examples for translations that we think are close to how we want it to translate and ask for the missing languages from the AI (based on the English + source code usage + 3-4 other correct translations)
(https://github.com/bitcoin/bitcoin/pull/33275#issuecomment-3246189935)
> Is there some additional context that translators have?
We could probably get more accurate translations by creating a bot that goes through each translation (<2000 values I think, we could batch them by files) fetches the source code context for the given translations and gives 3-4 examples for translations that we think are close to how we want it to translate and ask for the missing languages from the AI (based on the English + source code usage + 3-4 other correct translations)