oscoin / oscoin-parity-wasm-prototype

Prototype implemenation of Oscoin ledger on Parity Ethereum Wasm
0 stars 0 forks source link

Unlocking accounts race condition #13

Closed geigerzaehler closed 5 years ago

geigerzaehler commented 5 years ago

Unlocking node accounts concurrently may result in transactions being blocked indefinitely

Background

Currently the deployment and client code uses node accounts for signing transactions. This requires us to unlock the account before sending a transaction. We use web3::api::Personal::unlock_account for that. We call this method without the timeout parameter. This means that the the next transaction for the account submitted with the eth_sendTransaction RPC method will be signed.

If the eth_sendTransaction method is called without the sender account being unlocked on the node the RPC method will not respond until the transaction is manually confirmed through the node API. Since we are not manually confirming transactions the RPC method effectively blocks.

The Issue

The concrete issues arises if some code that unlocks an account and then sends a transaction is run concurrently. For example oscoin_deploy::deploy unlocks the dev account and then submits a contract creation transaction. If the end-to-end tests run two deploys in parallel then the following happens.

  1. Thread 1 calls unlock_account.
  2. Thread 2 calls unlock_account.
  3. Thread 1 receives unlock_account response and sends the contract creation transaction.
  4. Thread 2 receives unlock_account response and sends the contract creation transaction.
  5. The node receives the transaction from thread 1, process the transaction and locks the account again.
  6. The node receives the transaction from thread 2. The account is locked and the node does not send a response.

This means that one of the threads will block indefinitely.

Possible solutions

The easiest solution would be to add a timeout parameter to web3::api::Personal::unlock_account so that the account is unlocked for some time. The web3 package does not seem to support this properly. Passing the parameter results in an error.

To solve the issue properly we should move away from eth_sendTransaction and use eth_sendRawTransaction. This would require us to replace some of the web3 library code with our own code.

rockbmb commented 5 years ago

Closed, but spawned https://github.com/tomusdrw/rust-web3/issues/256.