cuprated: embeddable node library with Node::launch()#592
Conversation
c680c1c to
6542272
Compare
81d85e1 to
618b162
Compare
|
you should try and return something like I don't get the difference between or relative purposes of Node and NodeContext. Which should be used for what? with dry_run_check calling std::process::exit(code), won't an embedder calling config.dry_run_check() get their process killed? nit but SyncState renamed unnecessarily imo there's no way to shut it down? I like main.rs after this tho. I like CommandHandle, get_fast_sync_hashes, BlockchainManagerHandle. not an in depth review but these things stood out to me in a once-over and some tangential reading to try and understand the changes |
Deferred this to the shutdown PRs - they depend on this branch so they carry everything over (https://github.com/Cuprate/cuprate/pull/585/changes#diff-d448bb1addb0153e0d62b48a7e6531f9988ea8f1846ab783540eaf9f2c87d87fR159)
Maybe there could be a better name for each struct so its self-documenting?
Yeah, good call
It was a transitive change as i was also planning to add current sync target height to the struct, so the embedder can potentially create progress UIs/bars etc, got sidetracked though
Ty, added shutdown function to #585 (https://github.com/Cuprate/cuprate/pull/585/changes#diff-d448bb1addb0153e0d62b48a7e6531f9988ea8f1846ab783540eaf9f2c87d87fR342-R344) |
Gotcha, OK :) sorry for the incomplete review then, glad to hear most of those issues are resolved by later PRs |
|
cancelling workflow because this is some doc nits, let's think a little about our carbon footprint. |
|
Think this is the last change i want to make here, didn't like how Also added |
6b7516e to
71b6dcc
Compare
71b6dcc to
1caa5c5
Compare
1caa5c5 to
04a09db
Compare
388a728 to
5336303
Compare
…erHandle, doc nits
5336303 to
f6797b6
Compare
What
Extracts
cupratedinto an embeddable library (lib.rs/main.rspattern). Takes inspiration from the API proposed in #554Closes #516
Note to reviewers: This does not implement graceful shutdown / panic removal yet, those have been deferred to #585. For the final state with all changes, skip to #590
Why
To enable embedding the node in other applications
Where
cuprated:Cargo.toml- explicit[lib]+[[bin]]targetslib.rs(new) -Nodestruct,LaunchContextfor initialization items,Node::launch()main.rs- slimmed to a CLI wrapper overNode::launch()config.rs- decoupled from CLI args;find_config()returnsResult<Option<Config>>;dry_run_check()returnsVec<DryRunResult>blockchain.rs- newBlockchainInterfacecomposite handleblockchain/syncer.rs-SyncNotify->Syncer+SyncerHandleblockchain/interface.rs-COMMAND_TX/BLOCKS_BEING_HANDLEDstatics ->BlockchainManagerHandlesignals.rsandstatics.rsdeleted; state lives inNodeContextfor multi-instance useversion.rs-Defaultimplconsensus/fast-sync:FAST_SYNC_HASHESstatic +set_fast_sync_hashesremoved;fast_sync_stop_heightandvalidate_entriestake hashes as a parameter from LaunchContextHow
Node::launch(config)initializes everything and returns the embedder API:Internal state previously held in mutable statics lives in LaunchContext - the node is now safe to run multiple times in a single process.
BlockchainInterfacecomposes three handles into one type for ergonomics:Embedders use the public impl functions
node.blockchain.read()andnode.blockchain.context()dandelion.rspattern.