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

Refactor TxQueueManager and TxQueue #338

Closed
7 tasks
adambabik opened this issue Sep 18, 2017 · 1 comment
Closed
7 tasks

Refactor TxQueueManager and TxQueue #338

adambabik opened this issue Sep 18, 2017 · 1 comment
Assignees

Comments

@adambabik
Copy link
Contributor

adambabik commented Sep 18, 2017

Problem

As we moved the transaction queue from go-ethereum to status-go, we haven't refactored it. The code has some unused fragments and TxQueue is overly complicated.

Implementation

  • change package name to transaction,
  • get rid of common.QueuedTxID,
  • change NewTransactionQueue to NewQueue`,
  • remove setting handlers from StatusBackend.registerHandlers(),
  • move handlers from Queue to transaction.Manager,
  • CompleteTransaction divided into steps: get a transaction, mark it as being processed, verify params, perform actual sending,
  • there is a single method for sending transaction no matter if using upstream or not.

Acceptance Criteria

  1. Transaction Manager and Queue are easy to understand for newcomers,
  2. Unit tests coverage of the transaction package is at least 80%,
  3. Manager and Queue are thread-safe,
  4. Each method and non-obvious code fragments are properly commented.

Notes

Keep in mind that the queue manager and transaction queue must be thread-safe and when CompleteTransaction is called, a given transaction should be blocked from completing it again in the same time.

For easier testing, QueueManager should be configurable which errors are retriable.

Future Steps

TBD

@adambabik
Copy link
Contributor Author

adambabik commented Sep 22, 2017

Currently, we have two separate flows to send a transaction depending on which node is used (local vs remote). I will describe these two flows but I will skip transaction queuing.

Local node

  1. Call les.StatusBackend.SendTransaction with SendTxArgs and password,
  2. (inside SendTransaction) call b.EstimateGas and set the estimated gas if it's higher than defaultGas,
  3. (inside SendTransaction) call PublicTransactionPoolAPI.SendTransaction with possibly adjusted gas, SendTxArgs and password.
  4. (inside PublicTransactionPoolAPI.SendTransaction) get wallet using Backend.AccountManager().Find(),
  5. (inside PublicTransactionPoolAPI.SendTransaction) if nonce is nil, lock nonce,
  6. (inside PublicTransactionPoolAPI.SendTransaction) set some default values to SendTxArgs,
  7. (inside PublicTransactionPoolAPI.SendTransaction) sign transaction using wallet.SignTxWithPassphrase (there is also a condition for EIP155),
  8. (inside PublicTransactionPoolAPI.SendTransaction) call unexported submitTransaction which calls Backend.SendTx and returns hash (if contract is created it also prints its address).

Remote node

  1. Call eth_getTransactionCount using RPCClient,
  2. Parse arguments from SendTxArgs,
  3. Call eth_estimateGas using RPCClient,
  4. Create a new transaction using types.NewTransaction,
  5. Sign the transaction using types.SignTx,
  6. Encode transaction to bytes with rlp.EncodeToBytes,
  7. Call eth_sendRawTransaction.

Issues

As you can see, these two flows are different and as the differences are subtle, they should not exist.

Proposed solution

Create a proper function similar to PublicTransactionPoolAPI.SendTransaction in status-go and use it to send all transactions via eth_sendRawTransaction regardless which node is used.
✅ The logic is in one place, easy to test and fix issues.
❌ Not sure if all required go-ethereum structures are exported.

If it turns out not possible, we can create a service in go-ethereum which will expose a proper function. However, it should be independent of les and any other services. It should be possible to retrieve it with like this:

// "github.com/ethereum/go-ethereum/status"
var status *status.Backend
runningNode.Service(&status)

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