💬 theStack commented on issue "tracing: test `interface_usdt_net.py` fails due to garbage message type in `net:outbound_message` tracepoint":
(https://github.com/bitcoin/bitcoin/issues/33246#issuecomment-3246836456)
> Can you share the output of `readelf -n ./build/bin/bitcoind | grep outbound_message -A 5 -B 3`?
Sure:
```
$ readelf -n ./build/bin/bitcoind | grep outbound_message -A 5 -B 3
Arguments: -8@x21 8@x22 8@x23 4@x20 8@x0
stapsdt 0x00000057 NT_STAPSDT (SystemTap probe descriptors)
Provider: net
Name: outbound_message
Location: 0x00000000000cf3ac, Base: 0x00000000009ca828, Semaphore: 0x0000000000b19428
Arguments: -8@x21 8@x24 8@[sp, 136] 8@[x0] 8@x1 8@x2
st
...
(https://github.com/bitcoin/bitcoin/issues/33246#issuecomment-3246836456)
> Can you share the output of `readelf -n ./build/bin/bitcoind | grep outbound_message -A 5 -B 3`?
Sure:
```
$ readelf -n ./build/bin/bitcoind | grep outbound_message -A 5 -B 3
Arguments: -8@x21 8@x22 8@x23 4@x20 8@x0
stapsdt 0x00000057 NT_STAPSDT (SystemTap probe descriptors)
Provider: net
Name: outbound_message
Location: 0x00000000000cf3ac, Base: 0x00000000009ca828, Semaphore: 0x0000000000b19428
Arguments: -8@x21 8@x24 8@[sp, 136] 8@[x0] 8@x1 8@x2
st
...
💬 jonatack commented on issue "Old wallet support (Berkeley 4.8)":
(https://github.com/bitcoin/bitcoin/issues/33273#issuecomment-3246866424)
IIUC @jonasschnelli expressed a similar concern in https://github.com/bitcoin/bitcoin/issues/20160#issuecomment-712941676.
(https://github.com/bitcoin/bitcoin/issues/33273#issuecomment-3246866424)
IIUC @jonasschnelli expressed a similar concern in https://github.com/bitcoin/bitcoin/issues/20160#issuecomment-712941676.
💬 TheCharlatan commented on issue "Old wallet support (Berkeley 4.8)":
(https://github.com/bitcoin/bitcoin/issues/33273#issuecomment-3246901001)
> IIUC @jonasschnelli expressed a similar concern in #20160 (comment).
And the current code implements his suggested solution, so not sure this applies?
(https://github.com/bitcoin/bitcoin/issues/33273#issuecomment-3246901001)
> IIUC @jonasschnelli expressed a similar concern in #20160 (comment).
And the current code implements his suggested solution, so not sure this applies?
🤔 mzumsande reviewed a pull request: "index: Fix coinstats overflow"
(https://github.com/bitcoin/bitcoin/pull/30469#pullrequestreview-3178227240)
Did another round of review, and everything looked good to me.
I would prefer to have ef01a192d0ad821bb3d332ae69cd8b58c010b45d as a separate PR as suggested: As far as I understand it doesn't break compatibility and therefore doesn't need to be bundled with this.
Happy to A CK after this.
(https://github.com/bitcoin/bitcoin/pull/30469#pullrequestreview-3178227240)
Did another round of review, and everything looked good to me.
I would prefer to have ef01a192d0ad821bb3d332ae69cd8b58c010b45d as a separate PR as suggested: As far as I understand it doesn't break compatibility and therefore doesn't need to be bundled with this.
Happy to A CK after this.
💬 mzumsande commented on pull request "index: Fix coinstats overflow":
(https://github.com/bitcoin/bitcoin/pull/30469#discussion_r2317244666)
it would be good to add a little explanation to this commit 4a72b62a0d9345c1720de538cd3b42659f7d046b - maybe just saying that it's not practically relevant because of the huge reorgs it would require, but that `RevertBlock` should be consistent with `CustomAppend`.
(https://github.com/bitcoin/bitcoin/pull/30469#discussion_r2317244666)
it would be good to add a little explanation to this commit 4a72b62a0d9345c1720de538cd3b42659f7d046b - maybe just saying that it's not practically relevant because of the huge reorgs it would require, but that `RevertBlock` should be consistent with `CustomAppend`.
💬 jonatack commented on issue "Old wallet support (Berkeley 4.8)":
(https://github.com/bitcoin/bitcoin/issues/33273#issuecomment-3246955235)
@TheCharlatan
> Since the wallet files are precious data sources and could lead to stress when failing to load, I suggest to continue support for BDB wallets indefinitely.
(https://github.com/bitcoin/bitcoin/issues/33273#issuecomment-3246955235)
@TheCharlatan
> Since the wallet files are precious data sources and could lead to stress when failing to load, I suggest to continue support for BDB wallets indefinitely.
💬 w0xlt commented on pull request "wallet: Identify transactions spending 0-value outputs, and add tests for anchor outputs in a wallet":
(https://github.com/bitcoin/bitcoin/pull/33268#discussion_r2317289335)
nit:
```suggestion
bool CWallet::IsFromMe(const CTransaction& tx) const
{
return std::any_of(tx.vin.begin(), tx.vin.end(),
[this](const CTxIn& txin) { return WITH_LOCK(cs_wallet, return GetTXO(txin.prevout)); });
}
```
(https://github.com/bitcoin/bitcoin/pull/33268#discussion_r2317289335)
nit:
```suggestion
bool CWallet::IsFromMe(const CTransaction& tx) const
{
return std::any_of(tx.vin.begin(), tx.vin.end(),
[this](const CTxIn& txin) { return WITH_LOCK(cs_wallet, return GetTXO(txin.prevout)); });
}
```
💬 achow101 commented on pull request "wallet: Identify transactions spending 0-value outputs, and add tests for anchor outputs in a wallet":
(https://github.com/bitcoin/bitcoin/pull/33268#issuecomment-3246998303)
Didn't really like `m_from_me` commits, so switched the implementation to just cache "from me" rather than also storing it on disk.
(https://github.com/bitcoin/bitcoin/pull/33268#issuecomment-3246998303)
Didn't really like `m_from_me` commits, so switched the implementation to just cache "from me" rather than also storing it on disk.
💬 cryptomeow commented on pull request "Release: 30.0 translations update":
(https://github.com/bitcoin/bitcoin/pull/33275#issuecomment-3247028742)
Great! Regarding the Greek translation, I've gone one round and did some fixes based on the review and it certainly is helpful and provides actionable issues.
One false positive group of issues I noticed is the disagreement on translating terms.
For example one example is `Peers`, it seems the LLM insists on suggesting `Ομότιμοι` which is more about comparing people, while `Κόμβοι` is more commonly accepted in our case. Not sure how to address this so these issues don't show up, but fwiw I
...
(https://github.com/bitcoin/bitcoin/pull/33275#issuecomment-3247028742)
Great! Regarding the Greek translation, I've gone one round and did some fixes based on the review and it certainly is helpful and provides actionable issues.
One false positive group of issues I noticed is the disagreement on translating terms.
For example one example is `Peers`, it seems the LLM insists on suggesting `Ομότιμοι` which is more about comparing people, while `Κόμβοι` is more commonly accepted in our case. Not sure how to address this so these issues don't show up, but fwiw I
...
💬 achow101 commented on pull request "doc: unify `datacarriersize` warning with release notes":
(https://github.com/bitcoin/bitcoin/pull/33224#issuecomment-3247030103)
ACK 2885bd0e1c4fc863a7f28ff0fd353f5cffb03442
(https://github.com/bitcoin/bitcoin/pull/33224#issuecomment-3247030103)
ACK 2885bd0e1c4fc863a7f28ff0fd353f5cffb03442
🤔 w0xlt reviewed a pull request: "wallet: Identify transactions spending 0-value outputs, and add tests for anchor outputs in a wallet"
(https://github.com/bitcoin/bitcoin/pull/33268#pullrequestreview-3178375101)
ACK https://github.com/bitcoin/bitcoin/pull/33268/commits/69eeb11bc585e92a3b56811c3251f13dba05aa5f with non-blocking nits above.
(https://github.com/bitcoin/bitcoin/pull/33268#pullrequestreview-3178375101)
ACK https://github.com/bitcoin/bitcoin/pull/33268/commits/69eeb11bc585e92a3b56811c3251f13dba05aa5f with non-blocking nits above.
🚀 achow101 merged a pull request: "doc: unify `datacarriersize` warning with release notes"
(https://github.com/bitcoin/bitcoin/pull/33224)
(https://github.com/bitcoin/bitcoin/pull/33224)
🤔 w0xlt reviewed a pull request: "doc: truc packages allow sub min feerate transactions"
(https://github.com/bitcoin/bitcoin/pull/33220#pullrequestreview-3178384226)
ACK https://github.com/bitcoin/bitcoin/pull/33220/commits/b19fcdadb30bc61af5921f7da0f6e8ca68c1ff33
(https://github.com/bitcoin/bitcoin/pull/33220#pullrequestreview-3178384226)
ACK https://github.com/bitcoin/bitcoin/pull/33220/commits/b19fcdadb30bc61af5921f7da0f6e8ca68c1ff33
💬 w0xlt commented on pull request "doc: truc packages allow sub min feerate transactions":
(https://github.com/bitcoin/bitcoin/pull/33220#discussion_r2317348479)
nit:
```suggestion
submitted as a package. Note that this rule does not apply to
```
(https://github.com/bitcoin/bitcoin/pull/33220#discussion_r2317348479)
nit:
```suggestion
submitted as a package. Note that this rule does not apply to
```
👋 BenWestgate's pull request is ready for review: "doc: update multisig tutorial to use multipath descriptors"
(https://github.com/bitcoin/bitcoin/pull/33286)
(https://github.com/bitcoin/bitcoin/pull/33286)
💬 l0rinc commented on pull request "Release: 30.0 translations update":
(https://github.com/bitcoin/bitcoin/pull/33275#issuecomment-3247050743)
@cryptomeow you can check out this PR having the translationx in an XML format and feed the whole file to an LLM. http://aistudio.google.com let's you work with multiple complete translations. You could ask it to extract the key/value pairs you want and feed *that* to the other LLM of yours choice.
(https://github.com/bitcoin/bitcoin/pull/33275#issuecomment-3247050743)
@cryptomeow you can check out this PR having the translationx in an XML format and feed the whole file to an LLM. http://aistudio.google.com let's you work with multiple complete translations. You could ask it to extract the key/value pairs you want and feed *that* to the other LLM of yours choice.
💬 achow101 commented on pull request "ci: Migrate CI to hosted Cirrus Runners":
(https://github.com/bitcoin/bitcoin/pull/32989#issuecomment-3247120869)
ACK 3c5da69a232ba1cfb935012aa53e57002efe0d77
Not particularly familiar with github actions or the cirrus, but it seems like all of the tasks that we were running on cirrus previously are correctly ported over and working.
(https://github.com/bitcoin/bitcoin/pull/32989#issuecomment-3247120869)
ACK 3c5da69a232ba1cfb935012aa53e57002efe0d77
Not particularly familiar with github actions or the cirrus, but it seems like all of the tasks that we were running on cirrus previously are correctly ported over and working.
📝 Cunero opened a pull request: "Update ci.yml"
(https://github.com/bitcoin/bitcoin/pull/33287)
Ci whit actions roots
<!--
*** 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
* Any test improvements or new tests that improve coverage are always welcome.
* All other changes should have accompanying unit tests (see `src/test/`) or
functional tests (see `test/`). Con
...
(https://github.com/bitcoin/bitcoin/pull/33287)
Ci whit actions roots
<!--
*** 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
* Any test improvements or new tests that improve coverage are always welcome.
* All other changes should have accompanying unit tests (see `src/test/`) or
functional tests (see `test/`). Con
...
✅ Cunero closed a pull request: "Update ci.yml"
(https://github.com/bitcoin/bitcoin/pull/33287)
(https://github.com/bitcoin/bitcoin/pull/33287)
🤔 w0xlt reviewed a pull request: "doc: truc packages allow sub min feerate transactions"
(https://github.com/bitcoin/bitcoin/pull/33220#pullrequestreview-3178505708)
reACK https://github.com/bitcoin/bitcoin/pull/33220/commits/7270839af425adddb3ed436a58a41b5bc843dab5
(https://github.com/bitcoin/bitcoin/pull/33220#pullrequestreview-3178505708)
reACK https://github.com/bitcoin/bitcoin/pull/33220/commits/7270839af425adddb3ed436a58a41b5bc843dab5