💬 fanquake commented on issue "Building libsecp256k1 fails on OpenBSD 7.6":
(https://github.com/bitcoin/bitcoin/issues/31106#issuecomment-2418928156)
Yes, please move this upstream. Building libsecp standalone isn't supported inside this repository.
(https://github.com/bitcoin/bitcoin/issues/31106#issuecomment-2418928156)
Yes, please move this upstream. Building libsecp standalone isn't supported inside this repository.
💬 laanwj commented on issue "Distribute darknet node addresses via DNS seeds using AAAA records":
(https://github.com/bitcoin/bitcoin/issues/31062#issuecomment-2418932066)
Concept NACK
It's a clever trick, but we've considered this kind of thing before and it's a really a deep rabbit hole.
From what i heard it's not generally a good idea to stash different data in AAAA records. Caching DNS servers can have checks to assume the addresses are routable and valid. Additionally, there's also no guarantee that clients get all the addresses, so splitting the data is a bit brittle.
This would also break the assumption that a random address returned from the DNS s
...
(https://github.com/bitcoin/bitcoin/issues/31062#issuecomment-2418932066)
Concept NACK
It's a clever trick, but we've considered this kind of thing before and it's a really a deep rabbit hole.
From what i heard it's not generally a good idea to stash different data in AAAA records. Caching DNS servers can have checks to assume the addresses are routable and valid. Additionally, there's also no guarantee that clients get all the addresses, so splitting the data is a bit brittle.
This would also break the assumption that a random address returned from the DNS s
...
💬 laanwj commented on pull request "RFC: build: support for pre-compiled headers.":
(https://github.com/bitcoin/bitcoin/pull/31053#issuecomment-2418937798)
Concept ACK. It's nice that CMAKE proves a built-in abstraction for this so there's no need for really ugly stuff.
(https://github.com/bitcoin/bitcoin/pull/31053#issuecomment-2418937798)
Concept ACK. It's nice that CMAKE proves a built-in abstraction for this so there's no need for really ugly stuff.
💬 dergoegge commented on pull request "validation: Improve input script check error reporting":
(https://github.com/bitcoin/bitcoin/pull/31097#issuecomment-2418957328)
I don't know what to do about the CI failure. I guess we need a fix for #30960? In the mean time I can remove the test commit.
(https://github.com/bitcoin/bitcoin/pull/31097#issuecomment-2418957328)
I don't know what to do about the CI failure. I guess we need a fix for #30960? In the mean time I can remove the test commit.
💬 laanwj commented on issue "RFC: Formal description of the RPC API":
(https://github.com/bitcoin/bitcoin/issues/29912#issuecomment-2418963490)
i'm going to look into creating a [JSON schema](https://json-schema.org/) based specification for the RPC JSON interface. This is the same standard that [c-lightning uses](https://github.com/ElementsProject/lightning/tree/master/doc/schemas), so we could even share some of their code generation tooling (say, for generating Rust RPC bindings automatically, or a JSON-to-protobuf proxy :sweat_smile: ) and documentation generation (manpages, website).
This can, for large part, be exported mechani
...
(https://github.com/bitcoin/bitcoin/issues/29912#issuecomment-2418963490)
i'm going to look into creating a [JSON schema](https://json-schema.org/) based specification for the RPC JSON interface. This is the same standard that [c-lightning uses](https://github.com/ElementsProject/lightning/tree/master/doc/schemas), so we could even share some of their code generation tooling (say, for generating Rust RPC bindings automatically, or a JSON-to-protobuf proxy :sweat_smile: ) and documentation generation (manpages, website).
This can, for large part, be exported mechani
...
💬 0xB10C commented on pull request "rest: Support transaction broadcast in REST interface":
(https://github.com/bitcoin/bitcoin/pull/31065#issuecomment-2418999248)
One thing to consider is that this PR changes the REST interface from being read-only to also being able to "modify" node state (mempool in this case). This PR might introduce new considerations for someone exposing the REST interface to a local or even a public network: Someone who previously could only query information can now also publish transactions via the node.
(https://github.com/bitcoin/bitcoin/pull/31065#issuecomment-2418999248)
One thing to consider is that this PR changes the REST interface from being read-only to also being able to "modify" node state (mempool in this case). This PR might introduce new considerations for someone exposing the REST interface to a local or even a public network: Someone who previously could only query information can now also publish transactions via the node.
⚠️ cyphra opened an issue: "Spanish translation"
(https://github.com/bitcoin/bitcoin/issues/31107)
### Please describe the feature you'd like to see added.
[bitcoin_es.ts.gz](https://github.com/user-attachments/files/17409519/bitcoin_es.ts.gz)
### Is your feature related to a problem, if so please describe it.
_No response_
### Describe the solution you'd like
_No response_
### Describe any alternatives you've considered
_No response_
### Please leave any additional context
_No response_
(https://github.com/bitcoin/bitcoin/issues/31107)
### Please describe the feature you'd like to see added.
[bitcoin_es.ts.gz](https://github.com/user-attachments/files/17409519/bitcoin_es.ts.gz)
### Is your feature related to a problem, if so please describe it.
_No response_
### Describe the solution you'd like
_No response_
### Describe any alternatives you've considered
_No response_
### Please leave any additional context
_No response_
✅ fanquake closed an issue: "Spanish translation"
(https://github.com/bitcoin/bitcoin/issues/31107)
(https://github.com/bitcoin/bitcoin/issues/31107)
💬 fanquake commented on issue "Spanish translation":
(https://github.com/bitcoin/bitcoin/issues/31107#issuecomment-2419025234)
Thanks for helping with translations! However, changes to translations as well as new translations are submitted to [Bitcoin Core's Transifex page](https://www.transifex.com/projects/p/bitcoin/).
Translations are periodically pulled from Transifex and merged into the git repository. See the [translation process](https://github.com/bitcoin/bitcoin/blob/master/doc/translation_process.md) for details on how this works. So we do not accept translation changes as GitHub pull requests because the n
...
(https://github.com/bitcoin/bitcoin/issues/31107#issuecomment-2419025234)
Thanks for helping with translations! However, changes to translations as well as new translations are submitted to [Bitcoin Core's Transifex page](https://www.transifex.com/projects/p/bitcoin/).
Translations are periodically pulled from Transifex and merged into the git repository. See the [translation process](https://github.com/bitcoin/bitcoin/blob/master/doc/translation_process.md) for details on how this works. So we do not accept translation changes as GitHub pull requests because the n
...
💬 maflcko commented on pull request "validation: Improve input script check error reporting":
(https://github.com/bitcoin/bitcoin/pull/31097#issuecomment-2419059430)
`-par=1` (or whatever to make the thread inline) should work, no?
(https://github.com/bitcoin/bitcoin/pull/31097#issuecomment-2419059430)
`-par=1` (or whatever to make the thread inline) should work, no?
💬 1440000bytes commented on pull request "rest: Support transaction broadcast in REST interface":
(https://github.com/bitcoin/bitcoin/pull/31065#issuecomment-2419090597)
> One thing to consider is that this PR changes the REST interface from being read-only to also being able to "modify" node state (mempool in this case). This PR might introduce new considerations for someone exposing the REST interface to a local or even a public network: Someone who previously could only query information can now also publish transactions via the node.
- It improves privacy if a node broadcasts some transactions on behalf of others
- It is possible to modify the node state
...
(https://github.com/bitcoin/bitcoin/pull/31065#issuecomment-2419090597)
> One thing to consider is that this PR changes the REST interface from being read-only to also being able to "modify" node state (mempool in this case). This PR might introduce new considerations for someone exposing the REST interface to a local or even a public network: Someone who previously could only query information can now also publish transactions via the node.
- It improves privacy if a node broadcasts some transactions on behalf of others
- It is possible to modify the node state
...
👍 maflcko approved a pull request: "benchmark: Improve SipHash_32b accuracy to avoid potential optimization issues"
(https://github.com/bitcoin/bitcoin/pull/30349#pullrequestreview-2374747440)
lgtm ACK c49cc30b8185a50b4aa00cf705d313c8aa9482a3
(https://github.com/bitcoin/bitcoin/pull/30349#pullrequestreview-2374747440)
lgtm ACK c49cc30b8185a50b4aa00cf705d313c8aa9482a3
💬 maflcko commented on pull request "benchmark: Improve SipHash_32b accuracy to avoid potential optimization issues":
(https://github.com/bitcoin/bitcoin/pull/30349#discussion_r1804480124)
Maybe I am wrong, but in theory I think that a byte array has weaker memory alignment requirements than `uint64_t`. So this is (in a strict sense) undefined behavior anyway. I presume it could be tested by adding a padding byte to uint256 and compile with ubsan, and then run this bench to observe the failure.
(https://github.com/bitcoin/bitcoin/pull/30349#discussion_r1804480124)
Maybe I am wrong, but in theory I think that a byte array has weaker memory alignment requirements than `uint64_t`. So this is (in a strict sense) undefined behavior anyway. I presume it could be tested by adding a padding byte to uint256 and compile with ubsan, and then run this bench to observe the failure.
💬 laanwj commented on pull request "benchmark: Improve SipHash_32b accuracy to avoid potential optimization issues":
(https://github.com/bitcoin/bitcoin/pull/30349#discussion_r1804511824)
This is a great point. Instead of the type-pun, we'd likely want to make this safer by assigning the result to a `uint64_t` then memcpy it in instead. That likely isn't going to be significantly slower.
(https://github.com/bitcoin/bitcoin/pull/30349#discussion_r1804511824)
This is a great point. Instead of the type-pun, we'd likely want to make this safer by assigning the result to a `uint64_t` then memcpy it in instead. That likely isn't going to be significantly slower.
⚠️ maflcko opened an issue: "How to compile the GUI on opensuse tumbleweed with cmake?"
(https://github.com/bitcoin-core/gui/issues/842)
Looks like this broke after the cmake migration.
Steps to reproduce on a fresh `podman run -it --rm 'registry.opensuse.org/opensuse/tumbleweed:latest'`:
`zypper in -y awk qrencode libevent-devel boost-devel sqlite3-devel libqt5-qttools-devel libqt5-qtbase-devel libdb-4_8-devel find bison gcc-c++ libtool make autoconf automake python3 clang llvm lbzip2 patch xz curl wget htop git vim ccache && git clone https://github.com/bitcoin/bitcoin.git --depth=1 ./b-c && cd b-c && cmake -
...
(https://github.com/bitcoin-core/gui/issues/842)
Looks like this broke after the cmake migration.
Steps to reproduce on a fresh `podman run -it --rm 'registry.opensuse.org/opensuse/tumbleweed:latest'`:
`zypper in -y awk qrencode libevent-devel boost-devel sqlite3-devel libqt5-qttools-devel libqt5-qtbase-devel libdb-4_8-devel find bison gcc-c++ libtool make autoconf automake python3 clang llvm lbzip2 patch xz curl wget htop git vim ccache && git clone https://github.com/bitcoin/bitcoin.git --depth=1 ./b-c && cd b-c && cmake -
...
💬 maflcko commented on pull request "benchmark: Improve SipHash_32b accuracy to avoid potential optimization issues":
(https://github.com/bitcoin/bitcoin/pull/30349#discussion_r1804519365)
> Edit: Wait, this code is being removed. Sorry.
Yeah, sorry, I was just trying to come up with another reason to change the code here :sweat_smile:
(https://github.com/bitcoin/bitcoin/pull/30349#discussion_r1804519365)
> Edit: Wait, this code is being removed. Sorry.
Yeah, sorry, I was just trying to come up with another reason to change the code here :sweat_smile:
⚠️ maflcko opened an issue: "CI failure: `Error: Directory '/tmp/ccache_dir' must be created in advance.`"
(https://github.com/bitcoin/bitcoin/issues/31108)
This may happen when re-running the CI (which happens every week on every pull request).
Unfortunately there is no fix for this, other than a new push (normal push, force-push, rebase, ...), or to ignore the failure.
This happens after commit fa252da0b9cc6c7e795366ce4a1ddc4c198dff15, which modified the cirrus config.
(https://github.com/bitcoin/bitcoin/issues/31108)
This may happen when re-running the CI (which happens every week on every pull request).
Unfortunately there is no fix for this, other than a new push (normal push, force-push, rebase, ...), or to ignore the failure.
This happens after commit fa252da0b9cc6c7e795366ce4a1ddc4c198dff15, which modified the cirrus config.
💬 l0rinc commented on pull request "benchmark: Improve SipHash_32b accuracy to avoid potential optimization issues":
(https://github.com/bitcoin/bitcoin/pull/30349#discussion_r1804545063)
Do you have any concrete recommendation that I could apply here?
(https://github.com/bitcoin/bitcoin/pull/30349#discussion_r1804545063)
Do you have any concrete recommendation that I could apply here?
💬 maflcko commented on pull request "benchmark: Improve SipHash_32b accuracy to avoid potential optimization issues":
(https://github.com/bitcoin/bitcoin/pull/30349#discussion_r1804599437)
Sorry again, I am just trying to add useful context that hasn't been mentioned before. I just have an allergy against undefined behavior, so I wanted to mention that the previous code may have had one.
As a fun fact, I tried it myself and in theory the UB should look like:
```
src/bench/crypto_hash.cpp:198:9: runtime error: store to misaligned address 0x7fffdb31cb81 for type 'uint64_t' (aka 'unsigned long'), which requires 8 byte alignment
0x7fffdb31cb81: note: pointer points here
00 0
...
(https://github.com/bitcoin/bitcoin/pull/30349#discussion_r1804599437)
Sorry again, I am just trying to add useful context that hasn't been mentioned before. I just have an allergy against undefined behavior, so I wanted to mention that the previous code may have had one.
As a fun fact, I tried it myself and in theory the UB should look like:
```
src/bench/crypto_hash.cpp:198:9: runtime error: store to misaligned address 0x7fffdb31cb81 for type 'uint64_t' (aka 'unsigned long'), which requires 8 byte alignment
0x7fffdb31cb81: note: pointer points here
00 0
...
💬 l0rinc commented on pull request "benchmark: Improve SipHash_32b accuracy to avoid potential optimization issues":
(https://github.com/bitcoin/bitcoin/pull/30349#discussion_r1804609063)
Thank you for checking!
(https://github.com/bitcoin/bitcoin/pull/30349#discussion_r1804609063)
Thank you for checking!