Bitcoin Core Github
43 subscribers
122K links
Download Telegram
πŸ’¬ jonatack commented on pull request "net, refactor: extract Network and BIP155Network logic to node/network":
(https://github.com/bitcoin/bitcoin/pull/27385#discussion_r1211651578)
It wasn't necessary to do here (no global scope name collisions), but I think it would make sense. Per `doc/developer-notes.md`:

```
- Prefer `enum class` (scoped enumerations) over `enum` (traditional enumerations) where possible.

- *Rationale*: Scoped enumerations avoid two potential pitfalls/problems with
traditional C++ enumerations: implicit conversions to `int`, and name clashes
due to enumerators being exported to the surrounding scope.
```
πŸ’¬ fanquake commented on issue "Remove Ambiguity of Script ASM Hex and Decimal Integer Representations":
(https://github.com/bitcoin/bitcoin/issues/27795#issuecomment-1570173602)
Related to #7996.
πŸ’¬ jonatack commented on pull request "net, refactor: extract Network and BIP155Network logic to node/network":
(https://github.com/bitcoin/bitcoin/pull/27385#issuecomment-1570183892)
Rebased πŸ₯‰

@ccdle12 Thanks! Mind re-acking? As to motivation, I think it's a mix of both reasons you mentioned, with the first one being the direction to go when doing so -- see `doc/design/libraries.md` and merged changes like c741d748d4.
πŸ’¬ willcl-ark commented on pull request "ci: Enable float-divide-by-zero check":
(https://github.com/bitcoin/bitcoin/pull/27778#issuecomment-1570188617)
utACK fa3ab4520317f48d4700b81dab023c4e639bbd68

The changes are straight forward-enough and I agree with the rationale, but I've been unable to test them locally due to unrealted build errors:

<details>
<summary>Details</summary>

```log
Making install in src
make[1]: Entering directory '/home/will/src/bitcoin/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src'
make[2]: Entering directory '/home/will/src/bitcoin/ci/scratch/build/bitcoin-x86_64-pc-linux-gnu/src'
rm -f libtest_util.a
/u
...
πŸ’¬ jonatack commented on pull request "test: move remaining rand code from util/setup_common to util/random":
(https://github.com/bitcoin/bitcoin/pull/27425#issuecomment-1570229861)
Rebased for https://github.com/bitcoin/bitcoin/pull/27571 πŸ₯ˆ
πŸ’¬ hebasto commented on pull request "ci: Enable float-divide-by-zero check":
(https://github.com/bitcoin/bitcoin/pull/27778#issuecomment-1570241144)
> I've been unable to test them locally due to unrealted build errors:

Mind providing more detailed steps to reproduce the failure?
πŸ’¬ MarcoFalke commented on pull request "ci: Enable float-divide-by-zero check":
(https://github.com/bitcoin/bitcoin/pull/27778#issuecomment-1570253029)
It probably depends on the CPU model to reproduce the unrelated failure?
πŸ’¬ fanquake commented on pull request "ci: Enable float-divide-by-zero check":
(https://github.com/bitcoin/bitcoin/pull/27778#issuecomment-1570253322)
> Mind providing more detailed steps to reproduce the failure?

Clearly those failures are unrelated to this change.
πŸ’¬ fanquake commented on pull request "ci: Enable float-divide-by-zero check":
(https://github.com/bitcoin/bitcoin/pull/27778#issuecomment-1570261798)
Checked that this found the previously fixed related failure, on aarch64, running the ASAN job:
```bash
test/argsman_tests.cpp(169): Entering test case "util_CheckValue"
validation.cpp:2839:5: runtime error: division by zero
#0 0xaaaab0a4c198 in Chainstate::ConnectTip(BlockValidationState&, CBlockIndex*, std::shared_ptr<CBlock const> const&, ConnectTrace&, DisconnectedBlockTransactions&) src/validation.cpp:2839:5
#1 0xaaaab0a506a0 in Chainstate::ActivateBestChainStep(BlockValidation
...
πŸš€ fanquake merged a pull request: "ci: Enable float-divide-by-zero check"
(https://github.com/bitcoin/bitcoin/pull/27778)
πŸ’¬ Sjors commented on pull request "assumeutxo (2)":
(https://github.com/bitcoin/bitcoin/pull/27596#discussion_r1211763951)
> @Sjors this is bash :)

*bashes head against table*
πŸ’¬ fanquake commented on pull request "macOS: Bump minimum required runtime version and prepare for building with upstream LLVM":
(https://github.com/bitcoin/bitcoin/pull/27676#issuecomment-1570295375)
Note that you'l have to drop the `lazy_bind` check (we'll follow up with a `fixup_chains` test later). I've got a branch here with that change, which Guix builds successfully: https://github.com/fanquake/bitcoin/tree/27676_minus_lazy_bind.

Guix Build:
```bash
2da11646ff030cbd9207027e5af33ec77e8dc30fa4811fb47e6c2201da9b54e5 guix-build-36d66d7df78e/output/arm64-apple-darwin/SHA256SUMS.part
e1c5a81e97a942ace7c57fd7c12b8f45287939dd193418c435ac806d28d76b28 guix-build-36d66d7df78e/output/arm64
...
πŸ’¬ 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_r1211789138)
Good question. The Socks5 server we use in the test framework by default disconnects immediately after the handshake and sometimes would not register as a connected peer:

https://cirrus-ci.com/task/6075793026056192?logs=ci#L3820-L3830
πŸ’¬ pinheadmz commented on pull request "test: cover addrv2 anchors by adding TorV3 to CAddress in messages.py":
(https://github.com/bitcoin/bitcoin/pull/27452#issuecomment-1570323701)
Thanks for the review Will, nits addressed and pushed
πŸ’¬ torkelrogstad commented on issue "Indicate RBF replaceability, also after transactions have been confirmed ":
(https://github.com/bitcoin/bitcoin/issues/27794#issuecomment-1570339251)
Ok, thanks for the input. IMO what I've laid out here is something consumers of this API would care about. The Core API in general is very hard to work with, this being one example.
βœ… torkelrogstad closed an issue: "Indicate RBF replaceability, also after transactions have been confirmed "
(https://github.com/bitcoin/bitcoin/issues/27794)
πŸ’¬ hebasto commented on pull request "Add public Boost headers explicitly":
(https://github.com/bitcoin/bitcoin/pull/27783#discussion_r1211807185)
> Still, does using `multi_index_container_fwd.hpp` help at all?

No, it doesn't:
```
./txmempool.h:406:29: error: field β€˜mapTx’ has incomplete type β€˜CTxMemPool::indexed_transaction_set’ {aka β€˜boost::multi_index::multi_index_container<CTxMemPoolEntry, boost::multi_index::indexed_by<boost::multi_index::hashed_unique<mempoolentry_txid, SaltedTxidHasher>, boost::multi_index::hashed_unique<boost::multi_index::tag<index_by_wtxid>, mempoolentry_wtxid, SaltedTxidHasher>, boost::multi_index::ordered
...
πŸ’¬ sipa commented on issue "Indicate RBF replaceability, also after transactions have been confirmed ":
(https://github.com/bitcoin/bitcoin/issues/27794#issuecomment-1570343467)
@torkelrogstad I'm still confused about what you're trying to do. The only sensible answer to the question whether a confirmed transaction is replaceable is *no*. Whatever purpose you have for wondering about that, by the time a transaction confirms, it shouldn't matter anymore to anything.
πŸ’¬ brunoerg commented on issue "Erlay status in getpeerinfo":
(https://github.com/bitcoin/bitcoin/issues/26602#issuecomment-1570354537)
In order to test Erlay I've implemented this recently to help me with my tests, I can open a PR for that.
πŸ’¬ jonatack commented on pull request "Raise on invalid -debug and -loglevel config options":
(https://github.com/bitcoin/bitcoin/pull/27632#discussion_r1211826449)
Fixed, thank you.