add libnimbus_lc.a C library
#5122
Conversation
Add a new C library for processing light client data based on the Nimbus implementation. This can be used from other, non-Nimbus components.
|
macOS requires #5132 |
|
Whatever nitpicks one might have about the C implementation, this PR is almost pure addition rather than modification of existing functionality, and presents no new default-running attack surface, so worth merging and adjusting from there. |
|
I might have a few but I'll need a few days to review - no opinion on merge order |
| * of the API. Also, all following calls must come from the same thread as from | ||
| * which this call was done. | ||
| */ | ||
| void NimMain(void); |
There was a problem hiding this comment.
See https://github.com/status-im/nim-style-guide/blob/main/interop/async/asynclib.nim#L16 - NimMain should not be exposed to users of the library
| * @return `NULL` - If an error occurred. | ||
| */ | ||
| ETH_RESULT_USE_CHECK | ||
| ETHRandomNumber *ETHRandomNumberCreate(void); |
There was a problem hiding this comment.
RNG feels like something we should hide behind a generic light client context - ie no need for library consumer to know / care
There was a problem hiding this comment.
What if someone wants to run multiple light client engines in the same thread? e.g., for a dashboard that tracks both Goerli and Mainnet? Sharing the RNG would be great in that case. Or should it simply be set up as part of a library wide ETHContextCreate?
| ETH_RESULT_USE_CHECK | ||
| ETHBeaconState *ETHBeaconStateCreateFromSsz( | ||
| const ETHConsensusConfig *cfg, | ||
| const char *consensusVersion, |
There was a problem hiding this comment.
we don't need the version (we can deduce it from the header)
There was a problem hiding this comment.
That's only possible due to a coincidence and not generally applicable. The header is not necessarily at the same location forever.
| * | ||
| * @see https://github.com/ethereum/consensus-specs/blob/v1.4.0-beta.0/specs/phase0/beacon-chain.md#custom-types | ||
| */ | ||
| void ETHRootDestroy(ETHRoot *root); |
There was a problem hiding this comment.
it might be easier to pass these around by value
There was a problem hiding this comment.
Maybe. In the WASM case, can only pass cint/pointer, so there would still be the need for memory management in the client.
| ## * Data gas used. | ||
| execution[].data_gas_used.cint | ||
|
|
||
| func ETHExecutionPayloadHeaderGetExcessDataGas( |
There was a problem hiding this comment.
isn't cint too small for this? ie max 2**31?
There was a problem hiding this comment.
Yeah, same for Slot, in theory would have to return it as a pair of high/low cint, or 4x / 8x (or allocate + matching Destroy).
But for practical ranges, cint is good enough. If higher values are possible in practice, needs to be extended.
|
In terms of C API, I'm not necessarily convinced that we need this great level of detail to access the internal details of the Nimbus implementation (the stores, the RNG, etc etc) - ie the REST API has a an appropriate abstraction level in that it represents the data and the events necessary to build a consumer of the light client API, and it has the benefit of being familiar to the ecosystem and well documented. As such, when thinking about a light client data consumer, I envision they launch the light client via some One can also plausibly think that someone might not want the entire async stack but rather drive the application using well-known data: in this case, one would feed it the messages as they come off the libp2p stack instead but again forego the internal implementation details and rather focus on a high-level context that collects everything needed under an opaque type. Re tests, why the genesis.ssz and other binary files? these can trivially be generated and thus not bloat the git history (which unfortunately will have to stay due to this being merged already)... |
debugging leftovers in |
Add a new C library for processing light client data based on the Nimbus implementation. This can be used from other, non-Nimbus components.