Skip to content
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

Extend Wallet Layer to Support Transaction Creation #95

Closed
5 tasks done
KtorZ opened this issue Mar 20, 2019 · 1 comment
Closed
5 tasks done

Extend Wallet Layer to Support Transaction Creation #95

KtorZ opened this issue Mar 20, 2019 · 1 comment
Assignees

Comments

@KtorZ
Copy link
Member

KtorZ commented Mar 20, 2019

Context

We do have a wallet layer which is our main interface to deal with wallet operation and make modification on our various checkpoints.

This layer is missing the ability to submit new pending transactions and sign them!

Decision

Extend the WalletLayer API with a way to submit transaction and keep track of the pending transaction in the underlying checkpoints and database engine. It should make use of the extended interface for the network layer and the coin selection (or at least, a mock of it).

Note that, to keep enough flexibility and allow for easily implementing already existing requests from various users, we do want to keep the input selection, transaction signing and transaction submission three separate actions. API handlers will eventually put them all together.

Acceptance Criteria

  1. Transaction submission (of an already signed transaction)must be supported by the wallet layer
  2. Transaction signing must be supported by the wallet layer
  3. Coin selection must be supported by the wallet layer

Development Plan

  • Port things from cardano-sl and experiment
  • Simplify drastically
  • Connect with the WalletLayer
  • Re-add features that "were lost"
  • Test by comparing implementation with cardano-sl

PR

Number Base
#164 master
#166 master
#175 master
#177 master
#181 master
#182 KtorZ/95/keystore-in-db
#198 master
#203 master

QA

  • This one was quite ... huge in the end. Many things have been done, starting from extending the wallet layer (see haddock) with three functions:

    • createUnsignedTx: run a coin selection across the wallet to build a transaction from available funds
    • signTx: produce witnesses for a given coin selection, requires a private key to have been stored
    • submitTx: broadcast a transaction to the network
  • At the API-level, a transaction handler has been implemented to perform the three steps above in sequence. We may consider later to introduce new endpoints to perform only some of those steps.

  • Regarding the transaction itself, there are a few things which are network-specific and depends on the binary representation of a transaction. We have therefore introduced an abstraction 'TransactionLayer' which captures this specificity. Golden tests with cardano-sl have been added here and make sure that we do generate and encode transactions exactly like cardano-sl implementation would do it.

  • Note that, there's currently no integration tests which verifying the end-to-end process of creating a transaction from an API call and watching it being picked up by the wallet. My two cents on this: this isn't trivial to do as it requires us to be able to make transactions, and therefore, to redeem funds from a faucet address first! I'd therefore suggest to release the feature as "experimental" for now in the next pre-release, and have a dedicated milestone about doing this. Regarding priorities, since transactions are going to have a different format in the Rust node and, a slightly different logic regarding witnesses, it'd be better to perform integration tests directly with the rust node instead of loosing a week or two to enable this with the http bridge.

@piotr-iohk
Copy link
Contributor

lgtm... conditionally (well, ok, the scope was Extend Wallet Layer... and this is delivered so... ok... lgtm 👍 ).

However I cannot yet test it on testnet with actual public facing API endpoint (in spite it is already implemented). The blocking issue is mainly that GET addresses endpoint is not implemented, hence there is no way to send any funds to the wallet (as address is unknown).

Also as discussed on Slack, we need to figure out the level of support for Byron node. If Byron node is to be supported then integration tests for it (via cardano-http-bridge will need to be added as well, not only for Jormungandr, as suggested in the ticket)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants