Skip to content
This repository has been archived by the owner on Aug 2, 2021. It is now read-only.

Queue and persist pending blockchain interaction #2006

Open
ralph-pichler opened this issue Dec 3, 2019 · 3 comments · May be fixed by #2124
Open

Queue and persist pending blockchain interaction #2006

ralph-pichler opened this issue Dec 3, 2019 · 3 comments · May be fixed by #2124

Comments

@ralph-pichler
Copy link
Member

ralph-pichler commented Dec 3, 2019

To ensure that there are no nonce issues we want to allow at most one pending blockchain transaction at a time. This means transactions will have to be queued if another one is still pending. This queue should also be persisted, so it can resume after restarts.

Requirements:

  • As there are multiple transaction types (e.g. cashing out, depositing, withdrawal) the queue should be agnostic of that. Swap should be able to push a transaction requests into the (FIFO) queue.
  • The transaction should be signed and sent as soon as there is no other transaction pending.
  • Once the transaction has been mined Swap should be notified somehow.
  • This notification should occur even if the node was shutdown in-between the time of sending the transaction and it being mined.
  • If a transaction got queued but not executed prior to being shutdown it should automatically be back in the queue after restart. (optional)

Requirements that are part of other issues:

Design goals:

  • should be as decoupled from swap as possible (ideally no dependency on swap and testable in isolation)
  • different components should be able to use the same queue (necessary as even swap itself needs to send txs from the same account from both Swap and the CashoutProcessor)
  • should be exchangeable with a queue which can allow concurrent transactions (= should have a well defined interface)
  • should only care about sending transaction (or setting the nonce) and reliably notifying about the result even in case of restarts or network or backend problems (re-trying, etc. would be outside its responsibility).
  • the idea is that other place in the code which need to send transaction no longer need to be concerned with issue like io errors, network problems, restarts, etc.. They just queue the request and are guaranteed to be notified of its result at some point.

Related to #2005, #1929 and #1633.

@ralph-pichler ralph-pichler self-assigned this Jan 14, 2020
@ralph-pichler ralph-pichler moved this from Backlog go to Current sprint in Incentives Sprint planning Jan 21, 2020
@ralph-pichler ralph-pichler moved this from Current sprint to In progress in Incentives Sprint planning Jan 21, 2020
@ralph-pichler
Copy link
Member Author

ralph-pichler commented Jan 22, 2020

The concept has undergone many changes and this description is no longer accurate.

My current idea goes as follows:

The queue implements the TransactionProcessor interface. (An alternative implementation of that interface might do local nonce tracking and allow for concurrent requests).

// TransactionProcessor represents a queue for transactions sent from a single ethereum account
// its purpose is to ensure there are no nonce issues and that transaction initiators are notified of the result
// notifications are guaranteed to happen even across restarts and disconnects from the ethereum backend
type TransactionProcessor interface {
	// Start starts processing transactions if it is not already doing so
	Start()
	// Stop stops processing transactions if it is running
	// It will block until processing has terminated
	Stop()
	// QueueRequest adds a new request to be processed
	QueueRequest(request TransactionRequest)
	// SetSubscriber sets subscriber as the subscriber for the specified type
	SetSubscriber(ty reflect.Type, subscriber TransactionRequestSubscriber)
}

TransactionRequest is an interface implemented by all structs representing a request for
future transaction. It's only purpose for now is to actually send the transaction using a nonce, signer and backend determined by the TransactionProcessor.

// TransactionRequest is the interface for different types of transactions
type TransactionRequest interface {
	// SendTransaction should send the transaction using the backend and opts provided
	// opts may be modified, however From, Nonce and Signer must be left untouched
	// if the transaction is sent through other means opts.Nonce must be respected (if set to nil, the "pending" nonce must be used)
	SendTransaction(backend contract.Backend, opts *bind.TransactOpts) (common.Hash, error)
}

To get notified of the outcome of TransactionRequests a component must register itself as the TransactionRequestSubscriber for that TransactionRequests type. For now there are only 3 notifications. If any of the notification handlers return an error it does not count as delivered and the handler will be tried again later (even across restarts of the swarm node). If no subscriber is registered at the time of the notification it also does not count as delivered and will be retried when a subscriber is set for that type of TransactionRequest.

// TransactionRequestSubscriber needs to be implemented by structs which want to be notified for a specific TransactionRequest
// If the handler returns an error the notification does not count as delivered
type TransactionRequestSubscriber interface {
	// NotifyMined gets called when the transaction was detected as successfully mined for the first time
	NotifyMined(request TransactionRequest, receipt *types.Receipt) error
	// NotifySendFailed gets called if sending the transaction to the network failed
	NotifySendFailed(request TransactionRequest, err error) error
	// NotifyTimeout gets called if the transaction did not get confirmed within the timeout
	// The transaction might still get mined in the future (in that case NotifyMined will still be called) or it might be replaced by another
	NotifyTimeout(request TransactionRequest) error
}

To ensure requests and notifications don't get lost over restarts all requests in the queue, the status of the current transaction as well as all pending notifications are saved to the state store on any change (This implies that the TransactionProcessor must be initialised with a list of all possible TransactionRequest types as otherwise we could not deserialise them again).
A potential extension to this would be a NotifyConfirmation(request TransactionRequest, uint64 confirmations) function. An alternative design would be to just have a single Notify function for everything an allow different NotificationTypes implementing some common interface. In that case the notification part might be separable from the rest of the TransactionProcessor.

@Eknir
Copy link
Contributor

Eknir commented Feb 6, 2020

Related: #2005

@ralph-pichler
Copy link
Member Author

see #2005 (comment)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
No open projects
2 participants