-
Notifications
You must be signed in to change notification settings - Fork 101
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
client/asset/eth: Transaction History #2504
Conversation
08d8f5b
to
71e3560
Compare
71e3560
to
599802f
Compare
599802f
to
4441186
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a first pass.
I hope we can now get rid of our clunky nonce system. If we're tracking our own confirmed and unconfirmed txs, we can get our nonce from there instead of from an rpc provider.
type extendedWalletTx struct { | ||
mtx sync.RWMutex | ||
*asset.WalletTransaction | ||
// Confirmed will be set to true once the transaction has 3 confirmations. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wasn't two enough after the merge to ensure it cannot be double spent or am I imagining that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, seems like reorgs are highly unlikely in ETH.
client/asset/eth/eth.go
Outdated
if !errors.Is(err, asset.CoinNotFoundError) { | ||
w.log.Errorf("Error getting confirmations for pending tx %s: %v", txHash, err) | ||
} | ||
if time.Since(time.Unix(int64(pendingTx.TimeStamp), 0)) > time.Minute*3 && tip%4 == 0 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't see every block, especially when using an HTTP RPC provider, so tip%4
is going to be unpredictable. Additionally, block times on Polygon are about 2 seconds.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've just removed this additional check completely. It only helps to get rid of this transaction a bit earlier from the pending balance and the tx history which is not really crucial, and only in the case where this wallet has a bug and submits two transactions with the same nonce, or the user exports the wallet and does a transaction externally.
This diff updates the ETH wallet to support the asset.WalletHistorian interface. Since ETH RPC nodes do not support querying the transactions an account has made, the transactions are stored locally in a database. Initially they are stored with a block number of 0 and the max possible fees, but when the transaction is confirmed, these values are updated. The pending transaction tracking used for more accurate balance reporting is updated to use the same data as required for tracking pending transactions for the wallet history. Also, the database used for monitoring pending redemption transactions and resubmitting them is upgraded to be used for both that functionality and the new tx history functionality.
This diff updates the ETH wallet to support the
asset.WalletHistorian
interface. Since ETH RPC nodes do not support querying the transactions an account has made, the transactions are stored locally in a database. Initially they are stored with a block number of 0 and the max possible fees, but when the transaction is confirmed, these values are updated.The pending transaction tracking used for more accurate balance reporting is updated to use the same data as required for tracking pending transactions for the wallet history. Also, the database used for monitoring pending redemption transactions and resubmitting them is upgraded to be used for both that functionality and the new tx history functionality.