Bitcoin Core Github
44 subscribers
121K links
Download Telegram
📝 tdb3 opened a pull request: "rpc: add getorphantxs"
(https://github.com/bitcoin/bitcoin/pull/30793)
PR https://github.com/bitcoin/bitcoin/pull/30784 adds a test that excessively large transactions aren't added to the orphanage. It checks the debug.log for confirmation that the transaction was not added.

As discussed in comment https://github.com/bitcoin/bitcoin/pull/30784#pullrequestreview-2274470190, knowing about the state of the orphanage is currently limited.

This PR adds a new rpc, `getorphantxs`, that provides the caller with a list of orphan transactions.

Currently looking for
...
💬 tdb3 commented on pull request "test: add check that too large txs aren't put into orphanage":
(https://github.com/bitcoin/bitcoin/pull/30784#issuecomment-2325361225)
> RPC already has a way to get mempool transactions (e.g. `getrawmempool` and `getmempoolentry`), but as far as I can tell, not orphans. Unless I'm overlooking something (or there's a history I'm not aware of), might be nice to have a `getorphantxs` (returning orphan txids, etc.) and `getorphan "txid"` (details for an orphan). Orphans do expire relatively quickly, so results from these RPC would be pretty ephemeral, but it might still be worth knowing. I'm taking a look at implementation options
...
👋 hebasto's pull request is ready for review: "build: Fix linking for `fuzz` target when building with MSan"
(https://github.com/bitcoin/bitcoin/pull/30778)
💬 hebasto commented on pull request "build: Fix linking for `fuzz` target when building with MSan":
(https://github.com/bitcoin/bitcoin/pull/30778#issuecomment-2325363726)
> > review-only ACK [331e976](https://github.com/bitcoin/bitcoin/commit/331e9761b639f380c457582ae3c4b05e56ff02b0)
>
> I'm still working on it. MSan fuzzing CI job still fails when running locally.

The issue has been resolved. The PR description has been updated.

Ready for review and testing.
💬 andrewtoth commented on pull request "rpc: add getorphantxs":
(https://github.com/bitcoin/bitcoin/pull/30793#issuecomment-2325371764)
Can you expand on the motivation behind exposing this please? Why would someone want to use such an RPC?
💬 0xB10C commented on pull request "tracing: network connection tracepoints":
(https://github.com/bitcoin/bitcoin/pull/25832#discussion_r1741287188)
note to self: update the binary path to new cmake location
💬 0xB10C commented on pull request "doc: update documentation and scripts related to build directories":
(https://github.com/bitcoin/bitcoin/pull/30741#issuecomment-2325385681)
Concept ACK
💬 andrewtoth commented on pull request "validation: write chainstate to disk every hour":
(https://github.com/bitcoin/bitcoin/pull/30611#discussion_r1741358607)
Done.
💬 andrewtoth commented on pull request "validation: write chainstate to disk every hour":
(https://github.com/bitcoin/bitcoin/pull/30611#discussion_r1741358669)
Ok, done!
💬 andrewtoth commented on pull request "rpc, rest: Improve block rpc error handling, check header before attempting to read block data.":
(https://github.com/bitcoin/bitcoin/pull/30410#issuecomment-2325490746)
I'm unsure why CI is failing now...
📝 ludete opened a pull request: "fix use int32_t instead of int type for risczero compile "
(https://github.com/bitcoin/bitcoin/pull/30794)
describe as above

--------------

### Issue

https://github.com/bitcoin/bitcoin/issues/30747
💬 maflcko commented on issue "Risczero Fit":
(https://github.com/bitcoin/bitcoin/issues/30747#issuecomment-2325665028)
> `assumptions.h` compiles successfully, but fails with `static_assert(std::is_same_v<int, int32_t>);`

Ok, that means `int` has the correct width and thus the correct behavior, apart from being a separate type that the serialization templates can't deal with.
💬 maflcko commented on pull request "fix use int32_t instead of int type for risczero compile":
(https://github.com/bitcoin/bitcoin/pull/30794#issuecomment-2325676629)
Can you explain *why* this is correct and *why* it is needed in the pull request description? Otherwise, current and future reviewers of the change will have a hard time following it.

Maybe something along the lines:

> Some platforms may define `int` and `int32_t` to be different types of the same width. This means that `src/compat/assumptions.h` compiles fine, however the templated serialization code can not accept values of type `int`.
>
> Fix compilation on those platforms by seriali
...
💬 maflcko commented on pull request "doc: update documentation and scripts related to build directories":
(https://github.com/bitcoin/bitcoin/pull/30741#issuecomment-2325688677)
review ACK 6a68343ffbf3291eb243d90c00df50e672ff3944
💬 maflcko commented on pull request "build: Fix linking for `fuzz` target when building with MSan":
(https://github.com/bitcoin/bitcoin/pull/30778#discussion_r1741494414)
26c460aa8b5decfd08d931b9b3f80be5c13c7528:

Can you explain this a bit better? Why did it only fail on one CI config, and none of the others? Why was the error an msan runtime-error, as opposed to a link-time error?
💬 inyellowbus commented on issue "Testnet4 consensus failure due to timewarp related "softfork"":
(https://github.com/bitcoin/bitcoin/issues/30786#issuecomment-2325791855)
> `bitcoind -reindex` does the trick, and only takes a minute on this tiny chain fortunately.

Reindex doesn't work for me. After starting the node, everything still stacks on block 42335. Maybe I need to do something else before starting the reindex? Or register a connection to some explicit node?
💬 Sjors commented on issue "Testnet4 consensus failure due to timewarp related "softfork"":
(https://github.com/bitcoin/bitcoin/issues/30786#issuecomment-2325810622)
> everything still stacks on block 42335.

This is excepted. You'll have to wait for a miner to do the same. Welcome to the testnet4 UASF :-)
💬 hebasto commented on pull request "build: Fix linking for `fuzz` target when building with MSan":
(https://github.com/bitcoin/bitcoin/pull/30778#discussion_r1741597310)
> [26c460a](https://github.com/bitcoin/bitcoin/commit/26c460aa8b5decfd08d931b9b3f80be5c13c7528):
>
> Can you explain this a bit better?

I'm afraid I can't (

> Why did it only fail on one CI config, and none of the others? Why was the error an msan runtime-error, as opposed to a link-time error?

Those are the questions I asked myself :)

I'm not able to answer them at this moment. There are two reasons for that:
1. The current set of internal libraries, including `test_fuzz` and `
...
💬 josibake commented on pull request "kernel: Introduce initial C header API":
(https://github.com/bitcoin/bitcoin/pull/30595#issuecomment-2325848908)
Concept ACK

Also an implicit approach ACK despite not heavily reviewing the code (yet). I have been focusing on using the kernel library in proof of concept applications to get a better sense of how well the library works for downstream users and to hopefully uncover any pain points preemptively. A few of these projects are linked in the PR description.

Regarding a C header vs C++ header, thanks @ryanofsky for taking the time to explain your thought process. I think you raise some excellen
...
💬 Sjors commented on pull request "net, net_processing: additional and consistent disconnect logging":
(https://github.com/bitcoin/bitcoin/pull/28521#issuecomment-2325863585)
Rebased after #30750. It made the diff slightly smaller, since I had made the same changes on every line touched here.