New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Provide wallet 'plugin' architecture #1983

Merged
merged 31 commits into from Nov 19, 2018

Conversation

Projects
None yet
4 participants
@yeastplume
Member

yeastplume commented Nov 14, 2018

Still experimental, and still not entirely sure how to structure this.

Basically, we're looking at trying to provide easily-swappable implementations of an interface that provides 2 separate things:

  1. Handle transaction send commands from one wallet to another
  2. Handle the instantiation of a listener to receive commands from 1

1 is fairly straightforward, as it's just a simple call to something that either expects a response or doesn't, that's called by the wallet transaction logic similar to how it's being done now. We may want to break up the wallet api somewhat to ensure that transaction building functions don't send anything to anyone, but just return the result of the current step to pass on / respond as required (that will probably make the workflow a bit more flexible).

  1. is a little bit more awkward, as it's essentially running some sort of service listening somewhere. The interface function needs an instantiated wallet passed to it in order to operate.

Still trying to think this through. I've consolidated the relevant areas of logic into wallet/src/clients/http.rs as a start, though I'm thinking as per 1 above the WalletToWalletClient trait can probably go and be replaced with simple function pointer rather than needing to be a trait on the wallet, especially if the transaction logic is broken up a bit.

Any comments or ideas on how this should be structured are very welcome.

@hashmap

This comment has been minimized.

Member

hashmap commented Nov 14, 2018

I wonder if we could go a different way and provide our wallet as an embeddable lib (like sqlite or lmdb)

@ignopeverell

This comment has been minimized.

Member

ignopeverell commented Nov 14, 2018

Perhaps I'm missing something but regarding 2, should the wallet API really be responsible to listening? I would expect more of a receive type method and leave the listening logic to the API consumer.

@sesam

This comment has been minimized.

Contributor

sesam commented Nov 14, 2018

we should support the flexible wallet shown on grincon0 for ios, where transport and tx creation were fully decoupled.
So +1 for a lib with minimal deps, maybe without its own storage, to also be used by the lightest of light wallets.

Then, for 'grin wallet' and grin localhost wallet, they could share a 'grin wallet listen' process similar to what we have now, for performance/batched sends. A clean api between UI and wallet security critical code makes audit smoother and we can fuzz the critical api surface more intensively.

@yeastplume

This comment has been minimized.

Member

yeastplume commented Nov 14, 2018

@ignopeverell no, the api isn't and shouldn't be responsible for listening. In the current implementation there's a wrapper around the API that either starts it up in listener mode (with an http/json listener around it), or allows functions to be called directly (as when the command line wallet is invoked).

In the current wallet invocation, you start the listener, which invokes the listener API with a wallet instance. There's also an http client, that can talk to this instance (I've isolated them into a single file in this PR so far).

Now I'm thinking now of a scenario where there are several choices listener/client pairs that you can specify in the config file (so like, a 'plugin', if not exactly). The current functionality can be thought of as an http 'plugin' which handles the listening and the corresponding client calls. A keybase 'plugin' would replace the client with keybase chat calls, while the keybase 'listener' would similarly be invoked with a wallet instance, and would watch for keybase messages and invoke the wallet api directly.

So the concept of a 'listener' can vary widely depending on what the implementation is (watching chat, polling for files etc).. it would seem we need a place where this listener can be defined with an active wallet instance and call API functions directly. (As I've started in wallet/src/clients/http.rs)

@yeastplume

This comment has been minimized.

Member

yeastplume commented Nov 15, 2018

So first step anyhow that seems to makes sense is to completely decouple the tx building from send/receive logic. At the moment you have a function like 'send_tx_slate' that builds the transaction, exchanges it via http and completes it. Unfortunately, this means you need a separate function to send to self, and a separate function to output to a file, etc.

So I've broken this up into discrete steps in the api, and left the send/receive part up to the caller to complete. I'm trying to introduce the concept of a wallet 'adaptor', which should handle the send/receive logic. The call out to another http node becomes encapsulated in an adaptor trait, and I should be able to rework the output-to-file logic to be another instance of an adapter instead of having to call separate, special functions.

Still not sure how to handle listeners and receiving, but this is moving in the right direction now, I think.

@yeastplume yeastplume changed the title from [WIP] Provide wallet 'plugin' architecture to Provide wallet 'plugin' architecture Nov 19, 2018

@yeastplume yeastplume merged commit 5ba163f into mimblewimble:master Nov 19, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@yeastplume yeastplume deleted the yeastplume:wallet-plugin branch Nov 28, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment