WALLET_INTERFACE: live testnet syncs #1983
Conversation
fluidvanadium
changed the title
| impl zcash_wallet_interface::Wallet for ZingoWallet { | ||
| fn user_agent_id() -> zcash_wallet_interface::UserAgentId { | ||
| const AGENT_NAME: &str = "zingo_wallet"; | ||
| const VERSION: &str = "0.0.1"; |
There was a problem hiding this comment.
I advocated the removal of the foo.0 accessor.
There was a problem hiding this comment.
Good idea. Done!
|
|
||
| let mut client = { | ||
| // global configuration must be manually set *somewhere* | ||
| rustls::crypto::ring::default_provider().install_default(); |
There was a problem hiding this comment.
Where should this be called? Why?
Is this called on the last possible line before use?
There was a problem hiding this comment.
I'm referring to the previous line of code.
| }; | ||
|
|
||
| let lightd_info = client | ||
| .get_lightd_info(tonic::Request::new( |
| let birthday: zcash_primitives::consensus::BlockHeight = | ||
| lightd_info.block_height.try_into()?; | ||
| (chain_type, birthday) | ||
| }; |
There was a problem hiding this comment.
Why not make the RHS of this let statement be a Single Use Helper Fn or "SUHFn".
This would suck code out of the body of this biggie fn, force an explicit type signature, inspire a name for the helper itself, and generally be elegant IMHO.
There was a problem hiding this comment.
Also helper fns (as named items) are relatively easy to reference, and e.g. test.
There was a problem hiding this comment.
I will. But its another step.
| (chain_type, birthday) | ||
| }; | ||
|
|
||
| // this seems like a lot of set up. Do we really need all this right here?? |
There was a problem hiding this comment.
It seems to me that the wallet-related functionality defined from here to the end of this fn might be useful outside the context of server addition.
So why not factor this out as another fn?
fluidvanadium
changed the title
fluidvanadium
changed the title
| #[error("Wallet creation failed with >{0}<.")] | ||
| CreateLightClient(#[from] zingolib::lightclient::error::LightClientError), | ||
| #[error("Wallet creation failed with >{0}<.")] | ||
| StartSync(zingolib::lightclient::error::LightClientError), |
There was a problem hiding this comment.
How can adding a server result in a sync error? Are we implying that it automatically starts a sync process?
3 participants
Why:
The interface to zingolib is scattered over many modules. I wanted to re-imagine what the interface could look like.
What:
The sum of assumptions necessary to create a wallet with zingolib is contained in code here.
The only change made to the zingolib package so far is the correction of a chain_from_str function.
Tests:
So far, this creates a ZingoWallet.
And tests that it reports syncking a chain.
Notably, this test does not involve customized test behavior. It only uses the interface methods presented in zcash_wallet_interface.