Bitcoin Core Github
44 subscribers
121K links
Download Telegram
🤔 MarcoFalke reviewed a pull request: "BIP324: ElligatorSwift integrations"
(https://github.com/bitcoin/bitcoin/pull/27479#pullrequestreview-1492430513)
left some style nits, feel free to ignore, unless you retouch for other reasons.
💬 MarcoFalke commented on pull request "BIP324: ElligatorSwift integrations":
(https://github.com/bitcoin/bitcoin/pull/27479#discussion_r1238543437)
```suggestion
auto ret = key.EllSwiftCreate(MakeByteSpan(entropy));
```

style nit: Can use the alias which does the same, with the added bonus that the viewed data is const and future improvements to `MakeByteSpan` will benefit this call site as well.
💬 MarcoFalke commented on pull request "BIP324: ElligatorSwift integrations":
(https://github.com/bitcoin/bitcoin/pull/27479#discussion_r1238043422)
style-nit: Obviously doesn't matter here, but could use `reinterpret_cast` to guard against accidentally removing constness. (And for symmetry with `UCharCast(const std::byte*)`)

```suggestion
inline unsigned char* UCharCast(std::byte* c) { return reinterpret_cast<unsigned char*>(c); }
```
💬 MarcoFalke commented on pull request "BIP324: ElligatorSwift integrations":
(https://github.com/bitcoin/bitcoin/pull/27479#discussion_r1238546031)
```suggestion
auto ellswift = key.EllSwiftCreate(MakeByteSpan(ent32));
```
👍 dergoegge approved a pull request: "Remove the syscall sandbox"
(https://github.com/bitcoin/bitcoin/pull/27896#pullrequestreview-1497976492)
ACK 32e2ffc39374f61bb2435da507f285459985df9e
fanquake closed an issue: "rpc: signed-integer-overflow in analyzepsbt["estimated_feerate"]"
(https://github.com/bitcoin/bitcoin/issues/27913)
🚀 fanquake merged a pull request: "feerate: For GetFeePerK() return nSatoshisPerK instead of round trip through GetFee"
(https://github.com/bitcoin/bitcoin/pull/27914)
📝 MarcoFalke converted_to_draft a pull request: "util: Allow std::byte and char Span serialization"
(https://github.com/bitcoin/bitcoin/pull/27927)
Seems odd to require developers to cast all byte-like spans passed to serialization to `unsigned char`-spans. Fix that by introducing a helper to convert any byte-like span to a `std::byte`-span, which is then accepted by serialization. Finally, add tests and update the code from `MakeUCharSpan`/`MakeByteSpan` to just `Span` where possible.
🚀 fanquake merged a pull request: "net: remove unused `CConnmanTest`"
(https://github.com/bitcoin/bitcoin/pull/27957)
⚠️ fanquake opened an issue: "ci: failure in `wallet_basic.py --descriptors`"
(https://github.com/bitcoin/bitcoin/issues/27974)
https://cirrus-ci.com/task/5029024158711808?logs=ci#L2900
```bash
node0 2023-06-23T16:51:00.034382Z [scheduler] [wallet/sqlite.cpp:47] [TraceSqlCallback] [/tmp/cirrus-ci-build/ci/scratch/test_runner/test_runner_₿_🏃_20230623_164625/wallet_basic_242/node0/regtest/wallets/default_wallet/wallet.dat] SQLite Statement: INSERT or REPLACE into main values(?, ?)
test 2023-06-23T16:51:00.043000Z TestFramework (ERROR): Assertion failed
Traceback (most recent cal
...
⚠️ dergoegge opened an issue: "fuzz: Use-of-uninitialized-value in evutil_inet_pton"
(https://github.com/bitcoin/bitcoin/issues/27975)
See https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=56529.

We already have [workarounds](https://github.com/bitcoin/bitcoin/blob/80f04febbc6048f9da85b9289e0d433d1f164de0/src/test/fuzz/http_request.cpp#L37-L43) for similar issues in the `http_request` target, so this probably just needs another one of those.
👍 dergoegge approved a pull request: "MaybePunishNodeForTx: Remove unused message arg and logging"
(https://github.com/bitcoin/bitcoin/pull/27947#pullrequestreview-1498042410)
utACK 9fe5f6d5d1ec31ebfacfe23368f22c2a0b58832d
💬 vasild commented on pull request "p2p: Diversify automatic outbound connections with respect to networks":
(https://github.com/bitcoin/bitcoin/pull/27213#discussion_r1241879650)
> change this data struct to an array, add explicit numeric values to the Network enum (for index values)

That seems unnecessary, it is good to rely as less as possible on the actual values of the `Network` enums. That is - the code better not break if `NET_ONION` is not `3`, for example. Use them just as labels. `std::unordered_map` fits best with that - `std::unordered_map<Network, size_t>` would work even if `NET_*` are changed to be strings!

> ... protect the array with m_nodes_mutex t
...
💬 MarcoFalke commented on pull request "util: Allow std::byte and char Span serialization":
(https://github.com/bitcoin/bitcoin/pull/27927#issuecomment-1607060383)
Rebased on https://github.com/bitcoin/bitcoin/pull/27973 for now, please review that first.
💬 MarcoFalke commented on pull request "MaybePunishNodeForTx: Remove unused message arg and logging":
(https://github.com/bitcoin/bitcoin/pull/27947#issuecomment-1607064993)
lgtm ACK 9fe5f6d5d1ec31ebfacfe23368f22c2a0b58832d 🕚

<details><summary>Show signature</summary>

Signature:

```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: lgtm ACK 9fe5f6d5d1ec31eb
...
💬 fanquake commented on pull request "Autogen 25.x":
(https://github.com/bitcoin/bitcoin/pull/27938#issuecomment-1607066605)
Thanks for the pull request, however it's unlikely we are going to make significant (refactoring) changes to our autotools based build system at this point.
fanquake closed a pull request: "Autogen 25.x"
(https://github.com/bitcoin/bitcoin/pull/27938)
💬 tansanDOTeth commented on issue "Keep getting errors after a while of syncing":
(https://github.com/bitcoin/bitcoin/issues/27972#issuecomment-1607082182)
I tried reindexing since I posted this and I got this:

> 023-06-26T09:16:24Z UpdateTip: new best=00000000000000000c93572c7fcf0ccc927b5fb00039e90d1550327d6d83552f height=368600 version=0x00000003 log2_work=83.174292 tx=78966308 date='2015-08-06T04:29:51Z' progress=0.093234 cache=380.0MiB(2905123txo)
2023-06-26T09:16:25Z UpdateTip: new best=00000000000000000be13a49f9c0852b6c2c773efc816afadf4b2328bebc1b0f height=368601 version=0x00000003 log2_work=83.174322 tx=78967060 date='2015-08-06T04:36:12
...
💬 fanquake commented on issue "deadlock shutting down v25.0":
(https://github.com/bitcoin/bitcoin/issues/27965#issuecomment-1607090365)
@Crypt-iQ possibly similar to issues you've been seeing?
🚀 fanquake merged a pull request: "MaybePunishNodeForTx: Remove unused message arg and logging"
(https://github.com/bitcoin/bitcoin/pull/27947)