š¬ fjahr commented on pull request "refactor: Use our own implementation of urlDecode":
(https://github.com/bitcoin/bitcoin/pull/29904#discussion_r1571291302)
fixed
(https://github.com/bitcoin/bitcoin/pull/29904#discussion_r1571291302)
fixed
š hebasto opened a pull request: "test: Fix `test/streams_tests.cpp` compilation on SunOS / illumos"
(https://github.com/bitcoin/bitcoin/pull/29907)
On systems where `int8_t` is defined as `char`, the `{S,Uns}erialize(Stream&, signed char)` functions become undefined.
This PR resolves the issue by testing `{S,Uns}erialize(Stream&, int8_t)` instead.
No behavior change on systems where `int8_t` is defined as `signed char`, which is the case for most other systems.
Fixes https://github.com/bitcoin/bitcoin/issues/29884.
An alternative approach is mentioned in https://github.com/bitcoin/bitcoin/issues/29884#issuecomment-2058434577 as
...
(https://github.com/bitcoin/bitcoin/pull/29907)
On systems where `int8_t` is defined as `char`, the `{S,Uns}erialize(Stream&, signed char)` functions become undefined.
This PR resolves the issue by testing `{S,Uns}erialize(Stream&, int8_t)` instead.
No behavior change on systems where `int8_t` is defined as `signed char`, which is the case for most other systems.
Fixes https://github.com/bitcoin/bitcoin/issues/29884.
An alternative approach is mentioned in https://github.com/bitcoin/bitcoin/issues/29884#issuecomment-2058434577 as
...
š¬ hebasto commented on issue "`test/streams_tests.cpp` fails to compile on SunOS / illumos":
(https://github.com/bitcoin/bitcoin/issues/29884#issuecomment-2065265007)
> Happy to review a pull, if someone creates one.
Please see https://github.com/bitcoin/bitcoin/pull/29907.
(https://github.com/bitcoin/bitcoin/issues/29884#issuecomment-2065265007)
> Happy to review a pull, if someone creates one.
Please see https://github.com/bitcoin/bitcoin/pull/29907.
š¬ fjahr commented on pull request "index: race fix, lock cs_main while 'm_synced' is subject to change":
(https://github.com/bitcoin/bitcoin/pull/29867#discussion_r1571344947)
nit: I had to read this three times to get it right, but feel free to ignore unless you have to retouch.
I would change it to `[...], so commit the data that was indexed so far.`
(https://github.com/bitcoin/bitcoin/pull/29867#discussion_r1571344947)
nit: I had to read this three times to get it right, but feel free to ignore unless you have to retouch.
I would change it to `[...], so commit the data that was indexed so far.`
š¬ fjahr commented on pull request "index: race fix, lock cs_main while 'm_synced' is subject to change":
(https://github.com/bitcoin/bitcoin/pull/29867#issuecomment-2065283298)
Code review ACK 65951e0418c53cbbf30b9ee85e24ccaf729088a1
(https://github.com/bitcoin/bitcoin/pull/29867#issuecomment-2065283298)
Code review ACK 65951e0418c53cbbf30b9ee85e24ccaf729088a1
ā ļø hebasto opened an issue: "test: SegFault in `ismine_tests` on SunOS / illumos"
(https://github.com/bitcoin/bitcoin/issues/29908)
```
$ gdb -q --args ./src/test/test_bitcoin -t ismine_tests
Reading symbols from ./src/test/test_bitcoin...
(gdb) run
Starting program: /export/home/hebasto/git/bitcoin/build/src/test/test_bitcoin -t ismine_tests
[Thread debugging using libthread_db enabled]
warning: could not convert 'mutex_t' from the host encoding (ISO-8859-1) to UTF-32.
This normally should not happen, please file a bug report.
Running 1 test case...
[New Thread 1 (LWP 1)]
Thread 2 received signal SIGSEGV, Segmen
...
(https://github.com/bitcoin/bitcoin/issues/29908)
```
$ gdb -q --args ./src/test/test_bitcoin -t ismine_tests
Reading symbols from ./src/test/test_bitcoin...
(gdb) run
Starting program: /export/home/hebasto/git/bitcoin/build/src/test/test_bitcoin -t ismine_tests
[Thread debugging using libthread_db enabled]
warning: could not convert 'mutex_t' from the host encoding (ISO-8859-1) to UTF-32.
This normally should not happen, please file a bug report.
Running 1 test case...
[New Thread 1 (LWP 1)]
Thread 2 received signal SIGSEGV, Segmen
...
š¬ petertodd commented on pull request "policy: Enable full-rbf by default":
(https://github.com/bitcoin/bitcoin/pull/28132#issuecomment-2065525144)
On Thu, Apr 18, 2024 at 11:24:20AM -0700, Antoine Riard wrote:
> > An update to @petertodd's stats from July, at least 97.7 % of hashrate (monthly average) is now running FullRBF based on FullRBF'd transactions from within the past 24 hours.
>
> 97.7 % of hashrate is just overwhelming in matters of hashrate support for `mempoolfullrbf`. Iām still curious if anyone has data on the additional accumulated absolute fee traffic yielded by non-opting replacement that would have not propagated under l
...
(https://github.com/bitcoin/bitcoin/pull/28132#issuecomment-2065525144)
On Thu, Apr 18, 2024 at 11:24:20AM -0700, Antoine Riard wrote:
> > An update to @petertodd's stats from July, at least 97.7 % of hashrate (monthly average) is now running FullRBF based on FullRBF'd transactions from within the past 24 hours.
>
> 97.7 % of hashrate is just overwhelming in matters of hashrate support for `mempoolfullrbf`. Iām still curious if anyone has data on the additional accumulated absolute fee traffic yielded by non-opting replacement that would have not propagated under l
...
ā ļø kosuodhmwa opened an issue: "~/.bitcoin (which is a softlink to a separate vmware virtual drive) dir is now almost 1tb"
(https://github.com/bitcoin/bitcoin/issues/29909)
it's happened after i
- used version 27.x (built yesterday from master branch)
but i don't no it's because of new version 27.0 or not. (?)
**bitcoin.conf file:**
proxy=127.0.0.1:9050
listen=1
bind=127.0.0.1
server = 1
rpcallowip = 127.0.0.1
rpcport = 8332
rpcuser = myuser
rpcpass = mypass
txindex = 1
i was thinking it's enough to use 1tb drive for ~/.bitcoin directory (which contains the full block chain and other things)
thank you very much for you
...
(https://github.com/bitcoin/bitcoin/issues/29909)
it's happened after i
- used version 27.x (built yesterday from master branch)
but i don't no it's because of new version 27.0 or not. (?)
**bitcoin.conf file:**
proxy=127.0.0.1:9050
listen=1
bind=127.0.0.1
server = 1
rpcallowip = 127.0.0.1
rpcport = 8332
rpcuser = myuser
rpcpass = mypass
txindex = 1
i was thinking it's enough to use 1tb drive for ~/.bitcoin directory (which contains the full block chain and other things)
thank you very much for you
...
š¬ kosuodhmwa commented on issue "~/.bitcoin (which is a softlink to a separate vmware virtual drive) dir is now almost 1tb":
(https://github.com/bitcoin/bitcoin/issues/29909#issuecomment-2065556996)
that vmware virtual disk (where ~/.bitcoin dir is softlinked) is a separate physical sdd drive on win10 host (with vmware player free)

(https://github.com/bitcoin/bitcoin/issues/29909#issuecomment-2065556996)
that vmware virtual disk (where ~/.bitcoin dir is softlinked) is a separate physical sdd drive on win10 host (with vmware player free)

š¬ tdb3 commented on pull request "ZMQ: Support UNIX domain sockets":
(https://github.com/bitcoin/bitcoin/pull/27679#issuecomment-2065579070)
> crACK for [21d0e6c](https://github.com/bitcoin/bitcoin/commit/21d0e6c7b7c7af7f6e54a45829b4fbfba6923b86). Changes lgtm. Will follow up with some testing within the next few days as time allows.
re ACK for 21d0e6c7b7c7af7f6e54a45829b4fbfba6923b86
Retested the following:
- using "ipc:///tmp/mysocket" form in bitcoin.conf causes error and quit (as expected, since it's not unix:/)
- using "notvalid:///tmp/sock" in bitcoin.conf causes error and quit (as expected, since it's not unix:/)
-
...
(https://github.com/bitcoin/bitcoin/pull/27679#issuecomment-2065579070)
> crACK for [21d0e6c](https://github.com/bitcoin/bitcoin/commit/21d0e6c7b7c7af7f6e54a45829b4fbfba6923b86). Changes lgtm. Will follow up with some testing within the next few days as time allows.
re ACK for 21d0e6c7b7c7af7f6e54a45829b4fbfba6923b86
Retested the following:
- using "ipc:///tmp/mysocket" form in bitcoin.conf causes error and quit (as expected, since it's not unix:/)
- using "notvalid:///tmp/sock" in bitcoin.conf causes error and quit (as expected, since it's not unix:/)
-
...
š¬ mzumsande commented on pull request "rpc: Fixed signed integer overflow for large feerates":
(https://github.com/bitcoin/bitcoin/pull/29434#issuecomment-2065662650)
Looks like this has uncovered a bug in lnd, see https://twitter.com/roasbeef/status/1781086945986404559.
(https://github.com/bitcoin/bitcoin/pull/29434#issuecomment-2065662650)
Looks like this has uncovered a bug in lnd, see https://twitter.com/roasbeef/status/1781086945986404559.
š¤ josibake reviewed a pull request: "rename bitcoin.conf in dist tarball"
(https://github.com/bitcoin/bitcoin/pull/29903#pullrequestreview-2010758724)
ACK https://github.com/bitcoin/bitcoin/pull/29903/commits/7ee8680cfd97d55b8dde8df5372fae5d9cef39fa
(https://github.com/bitcoin/bitcoin/pull/29903#pullrequestreview-2010758724)
ACK https://github.com/bitcoin/bitcoin/pull/29903/commits/7ee8680cfd97d55b8dde8df5372fae5d9cef39fa
š¬ maflcko commented on pull request "rpc: Fixed signed integer overflow for large feerates":
(https://github.com/bitcoin/bitcoin/pull/29434#issuecomment-2066106153)
> a bug in btcd
I presume the bug was fixed in https://github.com/btcsuite/btcd/pull/2142
I wonder why this wasn't found earlier in btcd, because if signed integer overflow happened, it could have resulted in rejected transactions as well (or anything else, since it is undefined behavior).
A possible explanation could be that no one sent a large enough transaction, or set a large enough maxfeerate to trigger the UB, or maybe someone did run into it and then re-tried with different valu
...
(https://github.com/bitcoin/bitcoin/pull/29434#issuecomment-2066106153)
> a bug in btcd
I presume the bug was fixed in https://github.com/btcsuite/btcd/pull/2142
I wonder why this wasn't found earlier in btcd, because if signed integer overflow happened, it could have resulted in rejected transactions as well (or anything else, since it is undefined behavior).
A possible explanation could be that no one sent a large enough transaction, or set a large enough maxfeerate to trigger the UB, or maybe someone did run into it and then re-tried with different valu
...
š¤ Sjors reviewed a pull request: "refactor: Use our own implementation of urlDecode"
(https://github.com/bitcoin/bitcoin/pull/29904#pullrequestreview-2010862632)
ACK 68bf6c82fee917756b367db4e99a8f6bcb6476bf
Link to the code as of v2.2.12 which we use in depends:
* `evhttp_uridecode`: https://github.com/libevent/libevent/blob/release-2.1.12-stable/http.c#L3225-L3246
* `evhttp_decode_uri_internal`: https://github.com/libevent/libevent/blob/release-2.1.12-stable/http.c#L3169-L3205
(https://github.com/bitcoin/bitcoin/pull/29904#pullrequestreview-2010862632)
ACK 68bf6c82fee917756b367db4e99a8f6bcb6476bf
Link to the code as of v2.2.12 which we use in depends:
* `evhttp_uridecode`: https://github.com/libevent/libevent/blob/release-2.1.12-stable/http.c#L3225-L3246
* `evhttp_decode_uri_internal`: https://github.com/libevent/libevent/blob/release-2.1.12-stable/http.c#L3169-L3205
š¬ Sjors commented on pull request "refactor: Use our own implementation of urlDecode":
(https://github.com/bitcoin/bitcoin/pull/29904#discussion_r1572043682)
Was this doing anything, or did we forget to remove it earlier?
(https://github.com/bitcoin/bitcoin/pull/29904#discussion_r1572043682)
Was this doing anything, or did we forget to remove it earlier?
š¬ Sjors commented on pull request "refactor: Use our own implementation of urlDecode":
(https://github.com/bitcoin/bitcoin/pull/29904#discussion_r1572032575)
caa2040edc46291b3f032c64018eda2f4b31df39 nit: `auto [_, ec]`
(https://github.com/bitcoin/bitcoin/pull/29904#discussion_r1572032575)
caa2040edc46291b3f032c64018eda2f4b31df39 nit: `auto [_, ec]`
š¬ maflcko commented on issue "~/.bitcoin (which is a softlink to a separate vmware virtual drive) dir is now almost 1tb":
(https://github.com/bitcoin/bitcoin/issues/29909#issuecomment-2066122539)
Can you share the sizes of the largest folders?
I don't use `txindex` right now, but I could imagine that it requires several GB of space.
(https://github.com/bitcoin/bitcoin/issues/29909#issuecomment-2066122539)
Can you share the sizes of the largest folders?
I don't use `txindex` right now, but I could imagine that it requires several GB of space.
š¬ maflcko commented on issue "test: SegFault in `ismine_tests` on SunOS / illumos":
(https://github.com/bitcoin/bitcoin/issues/29908#issuecomment-2066136578)
Do the functional tests pass? Does it also happen in valgrind? Does it also happen in ubsan, asan, or tsan? Does it also happen when using a depends `DEBUG=1` and `--enable-debug` build?
(https://github.com/bitcoin/bitcoin/issues/29908#issuecomment-2066136578)
Do the functional tests pass? Does it also happen in valgrind? Does it also happen in ubsan, asan, or tsan? Does it also happen when using a depends `DEBUG=1` and `--enable-debug` build?
š¬ maflcko commented on pull request "test: Fix `test/streams_tests.cpp` compilation on SunOS / illumos":
(https://github.com/bitcoin/bitcoin/pull/29907#issuecomment-2066146100)
lgtm ACK 976e5d8f7b2bc77cb1443b8bf0f38cb07db70e9b
`int8_t` is also what real, non-test code should use.
(https://github.com/bitcoin/bitcoin/pull/29907#issuecomment-2066146100)
lgtm ACK 976e5d8f7b2bc77cb1443b8bf0f38cb07db70e9b
`int8_t` is also what real, non-test code should use.
š¬ maflcko commented on pull request "rpc: return warnings as an array instead of just a single one":
(https://github.com/bitcoin/bitcoin/pull/29845#discussion_r1572076435)
I'd say that a new field, named `node_warnings` would also be acceptable. There shouldn't be a need to keep the name in sync with the wallet warnings, as the wallet warnings are of a different topic than the node warnings.
But happy to re-ACK either approach.
(https://github.com/bitcoin/bitcoin/pull/29845#discussion_r1572076435)
I'd say that a new field, named `node_warnings` would also be acceptable. There shouldn't be a need to keep the name in sync with the wallet warnings, as the wallet warnings are of a different topic than the node warnings.
But happy to re-ACK either approach.