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

Asynchronous signing #636

Closed
gavofyork opened this issue May 20, 2017 · 9 comments
Closed

Asynchronous signing #636

gavofyork opened this issue May 20, 2017 · 9 comments
Labels

Comments

@gavofyork
Copy link

gavofyork commented May 20, 2017

Currently we have one standard API for sending a transaction: eth_sendTransaction. This accepts a transaction specification, constructs and signs a "raw" transaction (approving as necessary) and submits it to the network. It returns the hash of the submitted transaction.

This API is suboptimal: firstly, it combines a number of different activities. Signing is an operation fundamentally concerning the key-store; network submission merely concerns only the p2p networking subsystem. Secondly, an most importantly, it results in a potentially long RPC since user-approval could be a non-trivial task. Any APIs or logic that assume an RPC will complete in a reasonable period of time will break.

Proposal

Introduce three new APIs and deprecate eth_sign, eth_signTransaction, eth_sendTransaction.

  • eth_postSignTransaction(tx: TransactionSpec) => id: Number: Requests the construction and signing of tx. Returns the unique job identifier.
  • eth_postSignMessage(message: String, from: Address) => id: Number: Requests the prefixing and signing of message from identity from. Returns the unique job identifier.
  • eth_getResult(id: Number) => result | error: Returns the result of the previously requested job of identity id or an error if the job is not yet completed or resulted in an error.

eth_getResult would also be a subscribable endpoint within a pub/sub API, yet to be standardised.

Rationale

Jobs no longer hold up the RPC. Polling is a suboptimal solution, but no different than is currently supported in the eth_ RPC collection. This would transition well to a pub/sub mechanism (with the ability to subscribe to the result of a given job) when pub/sub becomes widespread and standardised.

@danfinlay
Copy link
Contributor

This looks like a good idea, I agree the current sendTransaction API is rough.

What if instead of returning an errorfor all non-signed states, the response could include some tx/message status in the result object, like this:

eth_getResult(idNumber, (err, result) => {

result === {
  status: 'pending approval' || 'transmitting to network',
  hash: '0x..',
  rawTx: '0x..', // why not return the raw tx while we're at it?
}
// true

})

To really flesh out the API, we should define the exact status codes/messages that would be returned by different phases, for cross-client compatibility. Here I'm borrowing transaction states tracked by MetaMask, they're probably similar across clients:

  • Signing Transaction
    • Pending User Approval
    • Approved, sending to network (with tx hash)
    • Sent to network, waiting for miners
    • Confirmed by miners
    • Failed (generic failure)
    • Rejected (user rejection)

Having written these out, I notice it might also be convenient for developers if this same eth_getResult API would return the mined transaction once it is mined with an updated status.

This way a single transaction-polling loop could provide a developer all the transaction state data that is relevant to them.

@Arachnid
Copy link
Contributor

Do the wire protocols already support multiple outstanding RPCs? If they do, I'd argue that it makes more sense to support long running requests at the wire level, rather than to layer a polling-based API on top of it.

@tomusdrw
Copy link

@Arachnid There is many issues you can run into with long running requests (most notably browser timeouts).
@FlySwatter Returning hash/rawtx for unconfirmed transactions might not be possible if the signer interface allows users to tune gas or gas price before signing. Second issue might be the nonce - if the requests can be signed in arbitrary order the nonce needs to be determined when the transaction is signed not when it's submitted.

@Arachnid
Copy link
Contributor

Arachnid commented May 22, 2017

@Arachnid There is many issues you can run into with long running requests (most notably browser timeouts).

Well, there's a websockets interface, too. And JS clients can set the desired timeout.

@tomusdrw
Copy link

With WebSockets interface it won't survive reconnect or page reload.

@danfinlay
Copy link
Contributor

Returning hash/rawtx for unconfirmed transactions might not be possible if the signer interface allows users to tune gas or gas price before signing. Second issue might be the nonce - if the requests can be signed in arbitrary order the nonce needs to be determined when the transaction is signed not when it's submitted.

Sure, so these could be optional parameters. Just a thought, it brings this polling spec a little closer to the eventual ideal behavior of a subscription to evolving transaction state.

@gavofyork
Copy link
Author

gavofyork commented May 23, 2017

i think there's something to be said for minimising the complexity of the RPC so we might not want to provide extraneous (through relevant) data without a decent reason.

for optional parameters, one potential landmine is that users and clients may not grasp when they should be made available, leading to different clients providing them in different situations and implementation lock-in/annoyed users.

definitely agree on standardising transitions; that said, if we split it out into two independent operations, then it's less of an issue in the RPC - metamask's middleware would be able to recombine as it sees fit.

@github-actions
Copy link

github-actions bot commented Jan 2, 2022

There has been no activity on this issue for two months. It will be closed in a week if no further activity occurs. If you would like to move this EIP forward, please respond to any outstanding feedback or add a comment indicating that you have addressed all required feedback and are ready for a review.

@github-actions github-actions bot added the stale label Jan 2, 2022
@github-actions
Copy link

This issue was closed due to inactivity. If you are still pursuing it, feel free to reopen it and respond to any feedback or request a review in a comment.

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

No branches or pull requests

4 participants