💬 MarcoFalke commented on pull request "util: Teach AutoFile how to XOR":
(https://github.com/bitcoin/bitcoin/pull/28060#discussion_r1268305587)
Maybe I'll pick up https://github.com/bitcoin/bitcoin/pull/28006 again and remove `Get`?
(https://github.com/bitcoin/bitcoin/pull/28060#discussion_r1268305587)
Maybe I'll pick up https://github.com/bitcoin/bitcoin/pull/28006 again and remove `Get`?
💬 jonatack commented on pull request "test: add end-to-end tests for CConnman and PeerManager":
(https://github.com/bitcoin/bitcoin/pull/26812#issuecomment-1642386101)
> needs rebase if still relevant.
For the `previous releases` CI? (I don't find any merge conflicts with current master.)
(https://github.com/bitcoin/bitcoin/pull/26812#issuecomment-1642386101)
> needs rebase if still relevant.
For the `previous releases` CI? (I don't find any merge conflicts with current master.)
💬 MarcoFalke commented on pull request "util: Teach AutoFile how to XOR":
(https://github.com/bitcoin/bitcoin/pull/28060#issuecomment-1642398008)
Pushed a small change to call `std::ftell` less often if a large data span is written. In a follow-up, changes can be made to call it less often when small data spans are written.
(https://github.com/bitcoin/bitcoin/pull/28060#issuecomment-1642398008)
Pushed a small change to call `std::ftell` less often if a large data span is written. In a follow-up, changes can be made to call it less often when small data spans are written.
💬 MarcoFalke commented on pull request "test: add end-to-end tests for CConnman and PeerManager":
(https://github.com/bitcoin/bitcoin/pull/26812#issuecomment-1642400630)
> I don't find any merge conflicts with current master.
Are you sure you ran a full build after the merge?
(https://github.com/bitcoin/bitcoin/pull/26812#issuecomment-1642400630)
> I don't find any merge conflicts with current master.
Are you sure you ran a full build after the merge?
💬 Crypt-iQ commented on pull request "util: Teach AutoFile how to XOR":
(https://github.com/bitcoin/bitcoin/pull/28060#discussion_r1268324286)
happy to review if you do
(https://github.com/bitcoin/bitcoin/pull/28060#discussion_r1268324286)
happy to review if you do
💬 pinheadmz commented on pull request "test: Add unit & functional test coverage for blockstore":
(https://github.com/bitcoin/bitcoin/pull/27850#discussion_r1268324570)
Thanks, just fixed & pushed
(https://github.com/bitcoin/bitcoin/pull/27850#discussion_r1268324570)
Thanks, just fixed & pushed
💬 pinheadmz commented on pull request "net: support unix domain sockets for -proxy and -onion":
(https://github.com/bitcoin/bitcoin/pull/27375#discussion_r1268326494)
ah of course thanks, fixed and pushed
(https://github.com/bitcoin/bitcoin/pull/27375#discussion_r1268326494)
ah of course thanks, fixed and pushed
💬 jonatack commented on pull request "test: add end-to-end tests for CConnman and PeerManager":
(https://github.com/bitcoin/bitcoin/pull/26812#issuecomment-1642405661)
Ah, the second try did it :)
<details><summary>build log</summary><p>
```
test/fuzz/process_message_e2e.cpp:59:1: error: a type specifier is required for all declarations
FUZZ_TARGET_INIT(process_message_e2e, initialize_process_message_e2e)
^
test/fuzz/process_message_e2e.cpp:59:18: error: use of undeclared identifier 'process_message_e2e'
FUZZ_TARGET_INIT(process_message_e2e, initialize_process_message_e2e)
^
test/fuzz/process_message_e2e.cpp:59:70: error: expected
...
(https://github.com/bitcoin/bitcoin/pull/26812#issuecomment-1642405661)
Ah, the second try did it :)
<details><summary>build log</summary><p>
```
test/fuzz/process_message_e2e.cpp:59:1: error: a type specifier is required for all declarations
FUZZ_TARGET_INIT(process_message_e2e, initialize_process_message_e2e)
^
test/fuzz/process_message_e2e.cpp:59:18: error: use of undeclared identifier 'process_message_e2e'
FUZZ_TARGET_INIT(process_message_e2e, initialize_process_message_e2e)
^
test/fuzz/process_message_e2e.cpp:59:70: error: expected
...
💬 Crypt-iQ commented on pull request "http: add evhttp_connection_set_closecb to avoid g_requests hang":
(https://github.com/bitcoin/bitcoin/pull/27909#discussion_r1268330073)
nice catch!
(https://github.com/bitcoin/bitcoin/pull/27909#discussion_r1268330073)
nice catch!
💬 Crypt-iQ commented on pull request "http: add evhttp_connection_set_closecb to avoid g_requests hang":
(https://github.com/bitcoin/bitcoin/pull/27909#issuecomment-1642413131)
> This assumes that a connection can never have more than 1 request, and I'm not sure if that's the case.
I'm also not sure. This PR body leads me to believe that it is possible to have more than 1 request per connection: https://github.com/libevent/libevent/pull/592#issue-292981989 , however I'm not sure whether it's possible for them to be interleaved and cause problems with your connection-based patch. Something I'll try to investigate.
(https://github.com/bitcoin/bitcoin/pull/27909#issuecomment-1642413131)
> This assumes that a connection can never have more than 1 request, and I'm not sure if that's the case.
I'm also not sure. This PR body leads me to believe that it is possible to have more than 1 request per connection: https://github.com/libevent/libevent/pull/592#issue-292981989 , however I'm not sure whether it's possible for them to be interleaved and cause problems with your connection-based patch. Something I'll try to investigate.
💬 pinheadmz commented on pull request "test: cover addrv2 anchors by adding TorV3 to CAddress in messages.py":
(https://github.com/bitcoin/bitcoin/pull/27452#discussion_r1268398896)
🤦 ty sir. fixed & pushed
(https://github.com/bitcoin/bitcoin/pull/27452#discussion_r1268398896)
🤦 ty sir. fixed & pushed
💬 jonatack commented on pull request "net, refactor: remove unneeded exports, use helpers over low-level code, use optional":
(https://github.com/bitcoin/bitcoin/pull/28078#discussion_r1268442591)
Good idea! Done.
(https://github.com/bitcoin/bitcoin/pull/28078#discussion_r1268442591)
Good idea! Done.
💬 jonatack commented on pull request "net, refactor: remove unneeded exports, use helpers over low-level code, use optional":
(https://github.com/bitcoin/bitcoin/pull/28078#discussion_r1268442671)
Done, changed from implicit in the declarations to explicit in the definitions.
(https://github.com/bitcoin/bitcoin/pull/28078#discussion_r1268442671)
Done, changed from implicit in the declarations to explicit in the definitions.
💬 jonatack commented on pull request "net, refactor: remove unneeded exports, use helpers over low-level code, use optional":
(https://github.com/bitcoin/bitcoin/pull/28078#discussion_r1268446564)
> nit: no need to specify where it's used for a public method imo
Done.
(https://github.com/bitcoin/bitcoin/pull/28078#discussion_r1268446564)
> nit: no need to specify where it's used for a public method imo
Done.
💬 jonatack commented on pull request "net, refactor: remove unneeded exports, use helpers over low-level code, use optional":
(https://github.com/bitcoin/bitcoin/pull/28078#discussion_r1268446978)
Dropped these changes.
(https://github.com/bitcoin/bitcoin/pull/28078#discussion_r1268446978)
Dropped these changes.
💬 jonatack commented on pull request "net, refactor: remove unneeded exports, use helpers over low-level code, use optional":
(https://github.com/bitcoin/bitcoin/pull/28078#discussion_r1268447455)
Dropped the snakecase updates.
(https://github.com/bitcoin/bitcoin/pull/28078#discussion_r1268447455)
Dropped the snakecase updates.
💬 jonatack commented on pull request "net, refactor: remove unneeded exports, use helpers over low-level code, use optional":
(https://github.com/bitcoin/bitcoin/pull/28078#issuecomment-1642518571)
Updated to take all feedback by @stickies-v and @vasild -- thank you!
(https://github.com/bitcoin/bitcoin/pull/28078#issuecomment-1642518571)
Updated to take all feedback by @stickies-v and @vasild -- thank you!
💬 jonatack commented on pull request "net, refactor: remove unneeded exports, use helpers over low-level code, use optional":
(https://github.com/bitcoin/bitcoin/pull/28078#discussion_r1268486082)
> Done, changed from implicit in the declarations to explicit in the definitions.
The CI is unhappy with that, so reverting the last commit to c69a74229da514228d3388e9652d2ea2e89224d7.
(https://github.com/bitcoin/bitcoin/pull/28078#discussion_r1268486082)
> Done, changed from implicit in the declarations to explicit in the definitions.
The CI is unhappy with that, so reverting the last commit to c69a74229da514228d3388e9652d2ea2e89224d7.
💬 jonatack commented on pull request "test: cover addrv2 anchors by adding TorV3 to CAddress in messages.py":
(https://github.com/bitcoin/bitcoin/pull/27452#issuecomment-1642547623)
ACK ba8ab4fc545800c4fb283a5ff0b19138a0451aba
(https://github.com/bitcoin/bitcoin/pull/27452#issuecomment-1642547623)
ACK ba8ab4fc545800c4fb283a5ff0b19138a0451aba
📝 mzumsande opened a pull request: "test: fix intermittent failure in wallet_resendwallettransactions.py"
(https://github.com/bitcoin/bitcoin/pull/28108)
Fixes #28094
The test bumps the mocktime for ~2 weeks and then triggers eviction from the mempool. But this bump will also cause a new resubmit, and if the timing is such that this resubmit happens right after the eviction and before the check that the tx was evicted, the test can fail as in #28094:
```
node0 2023-07-17T21:31:23.809483Z (mocktime: 2023-08-02T09:46:27Z) [httpworker.1] [validation.cpp:267] [LimitMempoolSize] [mempool] Expired 2 transactions from the memory pool
node0 2023-
...
(https://github.com/bitcoin/bitcoin/pull/28108)
Fixes #28094
The test bumps the mocktime for ~2 weeks and then triggers eviction from the mempool. But this bump will also cause a new resubmit, and if the timing is such that this resubmit happens right after the eviction and before the check that the tx was evicted, the test can fail as in #28094:
```
node0 2023-07-17T21:31:23.809483Z (mocktime: 2023-08-02T09:46:27Z) [httpworker.1] [validation.cpp:267] [LimitMempoolSize] [mempool] Expired 2 transactions from the memory pool
node0 2023-
...