📝 hebasto opened a pull request: "build: Fix building `fuzz` binary on on SunOS / illumos"
(https://github.com/bitcoin/bitcoin/pull/30216)
On master branch @ 457e1846d2bf6ef9d54b9ba1a330ba8bbff13091, building the `fuzz` binary fails:
```
$ ./autogen.sh
$ ./configure
$ gmake -C src test/fuzz/fuzz
< snip >
CXX test/fuzz/fuzz-http_request.o
test/fuzz/http_request.cpp:13:10: fatal error: event2/buffer.h: No such file or directory
13 | #include <event2/buffer.h>
| ^~~~~~~~~~~~~~~~~
compilation terminated.
gmake: *** [Makefile:17138: test/fuzz/fuzz-http_request.o] Error 1
gmake: Leaving directory '/ex
...
(https://github.com/bitcoin/bitcoin/pull/30216)
On master branch @ 457e1846d2bf6ef9d54b9ba1a330ba8bbff13091, building the `fuzz` binary fails:
```
$ ./autogen.sh
$ ./configure
$ gmake -C src test/fuzz/fuzz
< snip >
CXX test/fuzz/fuzz-http_request.o
test/fuzz/http_request.cpp:13:10: fatal error: event2/buffer.h: No such file or directory
13 | #include <event2/buffer.h>
| ^~~~~~~~~~~~~~~~~
compilation terminated.
gmake: *** [Makefile:17138: test/fuzz/fuzz-http_request.o] Error 1
gmake: Leaving directory '/ex
...
💬 laib200 commented on pull request "Fee Estimation: Ignore all transactions with an in-block child":
(https://github.com/bitcoin/bitcoin/pull/30079#issuecomment-2144007809)
براكة الله فيك 😊
(https://github.com/bitcoin/bitcoin/pull/30079#issuecomment-2144007809)
براكة الله فيك 😊
👍 tdb3 approved a pull request: "test: Validate oversized transactions or without inputs"
(https://github.com/bitcoin/bitcoin/pull/29862#pullrequestreview-2092614645)
re ACK for 0aa7db42564408edb41b0d42103d39ba4c2787dc
Re-ran modifications of the tests to induce failure as described in https://github.com/bitcoin/bitcoin/pull/29862#discussion_r1564345633 and https://github.com/bitcoin/bitcoin/pull/29862/commits/0aa7db42564408edb41b0d42103d39ba4c2787dc#r1578682415.
Unexpected reject reasons were caught (succesfully).
(https://github.com/bitcoin/bitcoin/pull/29862#pullrequestreview-2092614645)
re ACK for 0aa7db42564408edb41b0d42103d39ba4c2787dc
Re-ran modifications of the tests to induce failure as described in https://github.com/bitcoin/bitcoin/pull/29862#discussion_r1564345633 and https://github.com/bitcoin/bitcoin/pull/29862/commits/0aa7db42564408edb41b0d42103d39ba4c2787dc#r1578682415.
Unexpected reject reasons were caught (succesfully).
👍 tdb3 approved a pull request: "netbase: extend CreateSock() to support creating arbitrary sockets"
(https://github.com/bitcoin/bitcoin/pull/30202#pullrequestreview-2092618199)
ACK for 77e34ded543e19ba27cab8d0cfc464af27f04bbf
Thanks. Seems like a reasonable change.
Sanity checks performed:
- Synced the last 1.5 day's worth of blocks on signet. Binding and sync seemed to work fine.
- Ran all unit and functional tests (passed).
Left some minor notes (just thinking out loud, so no need to answer/comment unless you feel it's necessary).
(https://github.com/bitcoin/bitcoin/pull/30202#pullrequestreview-2092618199)
ACK for 77e34ded543e19ba27cab8d0cfc464af27f04bbf
Thanks. Seems like a reasonable change.
Sanity checks performed:
- Synced the last 1.5 day's worth of blocks on signet. Binding and sync seemed to work fine.
- Ran all unit and functional tests (passed).
Left some minor notes (just thinking out loud, so no need to answer/comment unless you feel it's necessary).
💬 tdb3 commented on pull request "netbase: extend CreateSock() to support creating arbitrary sockets":
(https://github.com/bitcoin/bitcoin/pull/30202#discussion_r1623667994)
Seems good to add this sanity check to prevent setting non-pertinent sockopts. Might be worthwhile to mention in the PR description that in addition to extending arguments, some extra safety checks were put in place.
(https://github.com/bitcoin/bitcoin/pull/30202#discussion_r1623667994)
Seems good to add this sanity check to prevent setting non-pertinent sockopts. Might be worthwhile to mention in the PR description that in addition to extending arguments, some extra safety checks were put in place.
💬 tdb3 commented on pull request "netbase: extend CreateSock() to support creating arbitrary sockets":
(https://github.com/bitcoin/bitcoin/pull/30202#discussion_r1623667512)
Seems ok to remove, since `protocol` is now a passthrough argument.
(https://github.com/bitcoin/bitcoin/pull/30202#discussion_r1623667512)
Seems ok to remove, since `protocol` is now a passthrough argument.
💬 tdb3 commented on pull request "netbase: extend CreateSock() to support creating arbitrary sockets":
(https://github.com/bitcoin/bitcoin/pull/30202#discussion_r1623665893)
I'm assuming changing `GetSAFamily()`'s return type was avoided to keep the PR scoped (and since the change doesn't seem necessary).
Since `GetSAFamily()` returns `sa_family_t`, looks like this is a simple promotion from an `unsigned short` (`sa_family_t`) to `int`. This seems fine (it aligns with the typical way `socket()` args are typically used).
(https://github.com/bitcoin/bitcoin/pull/30202#discussion_r1623665893)
I'm assuming changing `GetSAFamily()`'s return type was avoided to keep the PR scoped (and since the change doesn't seem necessary).
Since `GetSAFamily()` returns `sa_family_t`, looks like this is a simple promotion from an `unsigned short` (`sa_family_t`) to `int`. This seems fine (it aligns with the typical way `socket()` args are typically used).
💬 tdb3 commented on pull request "netbase: extend CreateSock() to support creating arbitrary sockets":
(https://github.com/bitcoin/bitcoin/pull/30202#discussion_r1623668596)
Since these are simple `int`s passed by value, seems ok to change from the previous const reference approach.
(https://github.com/bitcoin/bitcoin/pull/30202#discussion_r1623668596)
Since these are simple `int`s passed by value, seems ok to change from the previous const reference approach.
💬 furszy commented on pull request "wallet: fix unrelated parent conflict doesn't cause child tx to be marked as conflict":
(https://github.com/bitcoin/bitcoin/pull/29680#discussion_r1623680370)
Can use `pindexNew` hash. We cache the block hash there.
(https://github.com/bitcoin/bitcoin/pull/29680#discussion_r1623680370)
Can use `pindexNew` hash. We cache the block hash there.
💬 furszy commented on pull request "wallet: fix unrelated parent conflict doesn't cause child tx to be marked as conflict":
(https://github.com/bitcoin/bitcoin/pull/29680#discussion_r1623683248)
In b67c82a6f2ab6:
This commit does not compile, shouldn't have changed `m_all_conflicts` for `m_all_conflicting`.
(https://github.com/bitcoin/bitcoin/pull/29680#discussion_r1623683248)
In b67c82a6f2ab6:
This commit does not compile, shouldn't have changed `m_all_conflicts` for `m_all_conflicting`.
💬 furszy commented on pull request "wallet: fix unrelated parent conflict doesn't cause child tx to be marked as conflict":
(https://github.com/bitcoin/bitcoin/pull/29680#discussion_r1623681866)
Could replace all the `std::get_if` for a more readable `IsReason<Conflict>(&reason)` if you include this function:
```c++
template <typename T>
bool IsReason(const MemPoolRemovalReason& reason)
{
return std::get_if<T>(reason) != nullptr;
}
```
(https://github.com/bitcoin/bitcoin/pull/29680#discussion_r1623681866)
Could replace all the `std::get_if` for a more readable `IsReason<Conflict>(&reason)` if you include this function:
```c++
template <typename T>
bool IsReason(const MemPoolRemovalReason& reason)
{
return std::get_if<T>(reason) != nullptr;
}
```
💬 andrewtoth commented on pull request "validation: sync chainstate to disk after syncing to tip":
(https://github.com/bitcoin/bitcoin/pull/15218#discussion_r1623684934)
Updated the log message to be `"Node is still in IBD, rescheduling post-IBD chainstate disk sync..."`. Does that clarify it?
(https://github.com/bitcoin/bitcoin/pull/15218#discussion_r1623684934)
Updated the log message to be `"Node is still in IBD, rescheduling post-IBD chainstate disk sync..."`. Does that clarify it?
💬 andrewtoth commented on pull request "validation: sync chainstate to disk after syncing to tip":
(https://github.com/bitcoin/bitcoin/pull/15218#discussion_r1623684976)
Yes, good idea. Done.
(https://github.com/bitcoin/bitcoin/pull/15218#discussion_r1623684976)
Yes, good idea. Done.
💬 tdb3 commented on pull request "doc: add guidance for RPC to developer notes":
(https://github.com/bitcoin/bitcoin/pull/30142#discussion_r1623686633)
Thanks for taking a look.
Good point. Removed this statement as it is quite similar to `Try to make the RPC response a JSON object` and could be confusing.
(https://github.com/bitcoin/bitcoin/pull/30142#discussion_r1623686633)
Thanks for taking a look.
Good point. Removed this statement as it is quite similar to `Try to make the RPC response a JSON object` and could be confusing.
💬 tdb3 commented on pull request "doc: add guidance for RPC to developer notes":
(https://github.com/bitcoin/bitcoin/pull/30142#discussion_r1623686715)
Thanks. Agreed. Updated.
(https://github.com/bitcoin/bitcoin/pull/30142#discussion_r1623686715)
Thanks. Agreed. Updated.
💬 andrewtoth commented on pull request "validation: sync chainstate to disk after syncing to tip":
(https://github.com/bitcoin/bitcoin/pull/15218#issuecomment-2144181791)
@mzumsande @chrisguida thank you for your reviews and suggestions. I've addressed them and rebased.
(https://github.com/bitcoin/bitcoin/pull/15218#issuecomment-2144181791)
@mzumsande @chrisguida thank you for your reviews and suggestions. I've addressed them and rebased.
🚀 fanquake merged a pull request: "fuzz: increase `txorphan` harness stability"
(https://github.com/bitcoin/bitcoin/pull/30186)
(https://github.com/bitcoin/bitcoin/pull/30186)
📝 hebasto opened a pull request: "depends: Update Boost download link"
(https://github.com/bitcoin/bitcoin/pull/30217)
The Boost has [migrated](https://github.com/boostorg/boost-tasks/pull/3) their downloads from from boostorg.jfrog.io to archives.boost.io.
So do we.
FWIW, the download speed at my location is much better :)
(https://github.com/bitcoin/bitcoin/pull/30217)
The Boost has [migrated](https://github.com/boostorg/boost-tasks/pull/3) their downloads from from boostorg.jfrog.io to archives.boost.io.
So do we.
FWIW, the download speed at my location is much better :)
🚀 fanquake merged a pull request: "build: remove `--enable-lcov-branch-coverage`"
(https://github.com/bitcoin/bitcoin/pull/30192)
(https://github.com/bitcoin/bitcoin/pull/30192)
💬 fanquake commented on pull request "doc, rpc: Release notes and follow-ups for #29612":
(https://github.com/bitcoin/bitcoin/pull/30167#discussion_r1624101256)
I think we can make any further release note additions when the notes are consolidated / merged into the wiki.
(https://github.com/bitcoin/bitcoin/pull/30167#discussion_r1624101256)
I think we can make any further release note additions when the notes are consolidated / merged into the wiki.