💬 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!
💬 dergoegge commented on pull request "validation: Improve input script check error reporting":
(https://github.com/bitcoin/bitcoin/pull/31097#issuecomment-2419332156)
> -par=1 (or whatever to make the thread inline) should work, no?
That worked, thanks!
(https://github.com/bitcoin/bitcoin/pull/31097#issuecomment-2419332156)
> -par=1 (or whatever to make the thread inline) should work, no?
That worked, thanks!
💬 dergoegge commented on pull request "validation: Improve input script check error reporting":
(https://github.com/bitcoin/bitcoin/pull/31097#discussion_r1804632137)
Will do if i push again
(https://github.com/bitcoin/bitcoin/pull/31097#discussion_r1804632137)
Will do if i push again
👍 0xB10C approved a pull request: "rpc: net: follow-ups for #30062"
(https://github.com/bitcoin/bitcoin/pull/30183#pullrequestreview-2375071825)
Code Review ACK b33eb137e39c434a7be69e1453a708b0c52553c4
(https://github.com/bitcoin/bitcoin/pull/30183#pullrequestreview-2375071825)
Code Review ACK b33eb137e39c434a7be69e1453a708b0c52553c4