📝 theStack opened a pull request: "test: fix `AddNode` unit test failure on OpenBSD"
(https://github.com/bitcoin/bitcoin/pull/28891)
On OpenBSD 7.4, the following check of the unit test `test_addnode_getaddednodeinfo_and_connection_detection` currently fails:
```
BOOST_CHECK(!connman->AddNode({/*m_added_node=*/"127.1", /*m_use_v2transport=*/true}));
```
The reason for that is that this OS seemingly doesn't support the IPv4 shorthand notation with omitted zero-bytes:
```
$ ping 127.1
ping: no address associated with name
```
As a simple fix, this PR skips the check for this with a pre-processor #if. On NetBSD and
...
(https://github.com/bitcoin/bitcoin/pull/28891)
On OpenBSD 7.4, the following check of the unit test `test_addnode_getaddednodeinfo_and_connection_detection` currently fails:
```
BOOST_CHECK(!connman->AddNode({/*m_added_node=*/"127.1", /*m_use_v2transport=*/true}));
```
The reason for that is that this OS seemingly doesn't support the IPv4 shorthand notation with omitted zero-bytes:
```
$ ping 127.1
ping: no address associated with name
```
As a simple fix, this PR skips the check for this with a pre-processor #if. On NetBSD and
...
💬 theStack commented on pull request "test: fix `AddNode` unit test failure on OpenBSD":
(https://github.com/bitcoin/bitcoin/pull/28891#issuecomment-1814286184)
cc @pinheadmz who is apparently also running an OpenBSD server, as far as I remember :-)
(https://github.com/bitcoin/bitcoin/pull/28891#issuecomment-1814286184)
cc @pinheadmz who is apparently also running an OpenBSD server, as far as I remember :-)
✅ maflcko closed a pull request: "Sanitizing ports of -rpcconnect and -rpcport."
(https://github.com/bitcoin/bitcoin/pull/27820)
(https://github.com/bitcoin/bitcoin/pull/27820)
💬 ajtowns commented on pull request "Use LE hex-encoded representations in script ASM for pushed values <= 4 bytes":
(https://github.com/bitcoin/bitcoin/pull/28824#discussion_r1395576175)
Byte code `024000` is a minimal push of the two-byte vector `4000` (so passes `CheckMinimalPush`) but it's not a minimal `CScriptNum` since it just evaluates to 64, and should have been `0140` (so fails `fRequireMinimal` due to the trailing 0).
(https://github.com/bitcoin/bitcoin/pull/28824#discussion_r1395576175)
Byte code `024000` is a minimal push of the two-byte vector `4000` (so passes `CheckMinimalPush`) but it's not a minimal `CScriptNum` since it just evaluates to 64, and should have been `0140` (so fails `fRequireMinimal` due to the trailing 0).
✅ maflcko closed a pull request: "CLI: Only one Request Handler can be specified."
(https://github.com/bitcoin/bitcoin/pull/27815)
(https://github.com/bitcoin/bitcoin/pull/27815)
✅ maflcko closed a pull request: "Blocking arguments -nohelp, -noh, and -no?"
(https://github.com/bitcoin/bitcoin/pull/27814)
(https://github.com/bitcoin/bitcoin/pull/27814)
💬 maflcko commented on pull request "Blocking arguments -nohelp, -noh, and -no?":
(https://github.com/bitcoin/bitcoin/pull/27814#issuecomment-1814292353)
Closing for now, due to lack of activity. Let us know if you want to pick this up again, and if this should be reopened.
(https://github.com/bitcoin/bitcoin/pull/27814#issuecomment-1814292353)
Closing for now, due to lack of activity. Let us know if you want to pick this up again, and if this should be reopened.
💬 ajtowns commented on pull request "rpc: Remove deprecated -rpcserialversion":
(https://github.com/bitcoin/bitcoin/pull/28890#issuecomment-1814295998)
> It is deprecated
It's only been deprecated in master for a couple of months; better to wait for it to actually be deprecated in the current release for a little while to give people a chance to complain if they're still relying on this functionality?
Otherwise, changes here look like what I'd expect.
> If there is a use-case, likely a per-RPC flag can be added, if needed.
Could give bitcoin-util the capability to strip witness data from a block/tx and leave it up to users who need
...
(https://github.com/bitcoin/bitcoin/pull/28890#issuecomment-1814295998)
> It is deprecated
It's only been deprecated in master for a couple of months; better to wait for it to actually be deprecated in the current release for a little while to give people a chance to complain if they're still relying on this functionality?
Otherwise, changes here look like what I'd expect.
> If there is a use-case, likely a per-RPC flag can be added, if needed.
Could give bitcoin-util the capability to strip witness data from a block/tx and leave it up to users who need
...
💬 maflcko commented on pull request "rpc: Remove deprecated -rpcserialversion":
(https://github.com/bitcoin/bitcoin/pull/28890#issuecomment-1814309383)
> better to wait for it to actually be deprecated in the current release for a little while to give people a chance to complain if they're still relying on this functionality?
Happy to wait, but there shouldn't be any merge conflicts, so it should be trivial to just type `git revert` cleanly in the rare case that the deprecation will be kicked down the line one more release or so.
(https://github.com/bitcoin/bitcoin/pull/28890#issuecomment-1814309383)
> better to wait for it to actually be deprecated in the current release for a little while to give people a chance to complain if they're still relying on this functionality?
Happy to wait, but there shouldn't be any merge conflicts, so it should be trivial to just type `git revert` cleanly in the rare case that the deprecation will be kicked down the line one more release or so.
💬 hebasto commented on issue "build: Configuring with `-mno-sse4.1` does not fail the sse4.1 instrinsics check":
(https://github.com/bitcoin/bitcoin/issues/28864#issuecomment-1814316786)
FWIW, adding `-O1` to the test makes it fail with `CXXFLAGS="-mno-sse4.1"`.
(https://github.com/bitcoin/bitcoin/issues/28864#issuecomment-1814316786)
FWIW, adding `-O1` to the test makes it fail with `CXXFLAGS="-mno-sse4.1"`.
👋 TheCharlatan's pull request is ready for review: "refactor: Replace sets of txiter with CTxMemPoolEntryRefs"
(https://github.com/bitcoin/bitcoin/pull/28886)
(https://github.com/bitcoin/bitcoin/pull/28886)
💬 sipa commented on pull request "Use LE hex-encoded representations in script ASM for pushed values <= 4 bytes":
(https://github.com/bitcoin/bitcoin/pull/28824#discussion_r1395623337)
Yeah, there are two distinct concepts of minimality:
* Minimal pushes apply to the interpretation of stack elements as *byte vectors*, and are about whether you used the right opcode (OP_n, direct push, OP_PUSHDATA1, OP_PUSHDATA2, OP_PUSHDATA4): you have to use the first applicable one from that list for a push to be a minimal push.
* Minimally-encoded integers apply to the interpretation of stack elements as *numbers*, and are about whether no surplus bytes we introduced when encoding the num
...
(https://github.com/bitcoin/bitcoin/pull/28824#discussion_r1395623337)
Yeah, there are two distinct concepts of minimality:
* Minimal pushes apply to the interpretation of stack elements as *byte vectors*, and are about whether you used the right opcode (OP_n, direct push, OP_PUSHDATA1, OP_PUSHDATA2, OP_PUSHDATA4): you have to use the first applicable one from that list for a push to be a minimal push.
* Minimally-encoded integers apply to the interpretation of stack elements as *numbers*, and are about whether no surplus bytes we introduced when encoding the num
...
💬 TheCharlatan commented on pull request "RFC: Remove boost usage from kernel api / headers":
(https://github.com/bitcoin/bitcoin/pull/28335#discussion_r1395656090)
Added a new commit for this.
(https://github.com/bitcoin/bitcoin/pull/28335#discussion_r1395656090)
Added a new commit for this.
💬 maflcko commented on pull request "RFC: Remove boost usage from kernel api / headers":
(https://github.com/bitcoin/bitcoin/pull/28335#discussion_r1395662791)
Thanks, I guess it could be moved to https://github.com/bitcoin/bitcoin/pull/28886 as well, if you don't mind and have to retouch for other reasons.
(https://github.com/bitcoin/bitcoin/pull/28335#discussion_r1395662791)
Thanks, I guess it could be moved to https://github.com/bitcoin/bitcoin/pull/28886 as well, if you don't mind and have to retouch for other reasons.
💬 m3dwards commented on issue "26.0 RC Testing Guide Feedback":
(https://github.com/bitcoin/bitcoin/issues/28866#issuecomment-1814401058)
@D33r-Gee one of the pieces of feedback I received was to differentiate between when it was asking the user to enter a command and when it was showing the result of a command. It was suggested to use $ to make the distinction.
Noted your commend and will see what others think.
(https://github.com/bitcoin/bitcoin/issues/28866#issuecomment-1814401058)
@D33r-Gee one of the pieces of feedback I received was to differentiate between when it was asking the user to enter a command and when it was showing the result of a command. It was suggested to use $ to make the distinction.
Noted your commend and will see what others think.
💬 maflcko commented on pull request "refactor: Replace sets of txiter with CTxMemPoolEntryRefs":
(https://github.com/bitcoin/bitcoin/pull/28886#discussion_r1395665231)
```suggestion
for (const CTxMemPoolEntry& entry : GetEntrySet(setParentTransactions)) {
```
nit: Not a pointer?
(https://github.com/bitcoin/bitcoin/pull/28886#discussion_r1395665231)
```suggestion
for (const CTxMemPoolEntry& entry : GetEntrySet(setParentTransactions)) {
```
nit: Not a pointer?
💬 TheCharlatan commented on pull request "refactor: Replace sets of txiter with CTxMemPoolEntryRefs":
(https://github.com/bitcoin/bitcoin/pull/28886#discussion_r1395671439)
The `p` is for `parent`? Just like before in `pit`?
(https://github.com/bitcoin/bitcoin/pull/28886#discussion_r1395671439)
The `p` is for `parent`? Just like before in `pit`?
💬 fanquake commented on pull request "build: switch to using LLVM 17.x for macOS builds":
(https://github.com/bitcoin/bitcoin/pull/28880#issuecomment-1814426649)
> Guix hashes for arm64-apple-darwin differs across the build platforms, unfortunately.
Looks like the difference is only in `bitcoin-qt`. I'll check if reverting #28778 (dropping the -O1 workaround) makes things deterministic again. That would be disappointing, and a bit confusing, because it'd mean that things were non-deterministic, then became deterministic with newer Clang, and then went non-deterministic again with an even newer Clang..
(https://github.com/bitcoin/bitcoin/pull/28880#issuecomment-1814426649)
> Guix hashes for arm64-apple-darwin differs across the build platforms, unfortunately.
Looks like the difference is only in `bitcoin-qt`. I'll check if reverting #28778 (dropping the -O1 workaround) makes things deterministic again. That would be disappointing, and a bit confusing, because it'd mean that things were non-deterministic, then became deterministic with newer Clang, and then went non-deterministic again with an even newer Clang..
🤔 stickies-v reviewed a pull request: "build: Introduce internal kernel library"
(https://github.com/bitcoin/bitcoin/pull/28690#pullrequestreview-1734365185)
Concept ACK
It turned out to be a bit less clean than I'd hoped, but I made this branch with a scripted-diff (with a second commit to re-order includes alphabetically) as an alternative to the manual first commit. I'm not sure it makes reviewing easier (and it needs some more manual edits to Makefile, ubsan and an unrelated include), but at least it's a helpful check:
https://github.com/bitcoin/bitcoin/compare/master...stickies-v:bitcoin:182d54aed91bfb1c161033642c322a542c8f2e04
```sh
%
...
(https://github.com/bitcoin/bitcoin/pull/28690#pullrequestreview-1734365185)
Concept ACK
It turned out to be a bit less clean than I'd hoped, but I made this branch with a scripted-diff (with a second commit to re-order includes alphabetically) as an alternative to the manual first commit. I'm not sure it makes reviewing easier (and it needs some more manual edits to Makefile, ubsan and an unrelated include), but at least it's a helpful check:
https://github.com/bitcoin/bitcoin/compare/master...stickies-v:bitcoin:182d54aed91bfb1c161033642c322a542c8f2e04
```sh
%
...
💬 fanquake commented on pull request "build: switch to using LLVM 17.x for macOS builds":
(https://github.com/bitcoin/bitcoin/pull/28880#issuecomment-1814434680)
The diff I see between the two bins is:
```diff
--- a.txt
+++ b.txt
@@ -1,8 +1,8 @@
-bitcoin-f387bc0783ba_aarch64/bin/bitcoin-qt:
+bitcoin-f387bc0783ba_x86_64/bin/bitcoin-qt:
(__TEXT,__text) section
0000000100017364 bti c
0000000100017368 sub sp, sp, #0x30
000000010001736c stp x20, x19, [sp, #0x10]
0000000100017370 stp x29, x30, [sp, #0x20]
0000000100017374 add x29, sp, #0x20
0000000100017378 mov x19, x0
@@ -2987768,31 +2987768,31 @@
0000000100b7cf38 sub w9, w9, w8
00000
...
(https://github.com/bitcoin/bitcoin/pull/28880#issuecomment-1814434680)
The diff I see between the two bins is:
```diff
--- a.txt
+++ b.txt
@@ -1,8 +1,8 @@
-bitcoin-f387bc0783ba_aarch64/bin/bitcoin-qt:
+bitcoin-f387bc0783ba_x86_64/bin/bitcoin-qt:
(__TEXT,__text) section
0000000100017364 bti c
0000000100017368 sub sp, sp, #0x30
000000010001736c stp x20, x19, [sp, #0x10]
0000000100017370 stp x29, x30, [sp, #0x20]
0000000100017374 add x29, sp, #0x20
0000000100017378 mov x19, x0
@@ -2987768,31 +2987768,31 @@
0000000100b7cf38 sub w9, w9, w8
00000
...