immutability-io / vault-ethereum

A plugin that turns Vault into an Ethereum wallet.
244 stars 66 forks source link

Endpoint for signing transactions in json format #23

Closed DeRain closed 6 years ago

DeRain commented 6 years ago

Description

Brand new API endpoint for making signing transaction easy!

Motivation and Context

I'm tried common sign method, but it seems it's working only for custom data signatures. Anyway - I think we need a more developer's friendly way to sign the transaction.

The developer only needs to provide all json properties of the transaction and then he gets signed transaction in HEX format that ready to be broadcasted to Ethereum network by any convenient way. For example: Etherscan tool How to broadcast transaction via web3

How Has This Been Tested?

This is a brand new API and does not impact any existing code. I've created signed transactions and broadcasted them on Ropsten network via Etherscan publishing tool

Types of changes

Checklist:

cypherhat commented 6 years ago

Thinking about this... 2 things:

  1. Have you tested with both contract deploy transactions and send ETH transactions?
  2. chain_id shouldn't be an input parameter. The mount point is configured for a particular chain_id.

So, if you change:

_, err := b.configured(ctx, req)

to

config, err := b.configured(ctx, req)

...

chainID := ValidNumber(config.ChainID)
if chainID == nil {
    return nil, fmt.Errorf("invalid chain ID")
}

Then I think we've got something.

cypherhat commented 6 years ago

Actually, upon second thought, I am gonna do something a little different. We already have a method that signs a transaction - debit. The only thing we need to accommodate your ask is to:

  1. Render the transaction Hex in the output
  2. Add a flag to the method that prevents the transaction from being sent.

I am gonna try this.

cypherhat commented 6 years ago

I believe that this https://github.com/immutability-io/vault-ethereum/pull/24 addresses your request. Please feel free to comment here or there. But I will close this PR for now.

DeRain commented 6 years ago

@cypherhat I think that send(true|false)options to existing methods is fine, but in my opinion, we still need a more flexible method for signing transactions. For example, we might need a bunch of transaction with different smart contracts iteration, that causes custom tx_data in each case.

I've updated fork according to latest master and created a merge request with config updates. What do you think? https://github.com/immutability-io/vault-ethereum/pull/26