-
Notifications
You must be signed in to change notification settings - Fork 635
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
Improve nonce management logic for private mempool handling #6277
Conversation
@@ -148,6 +148,7 @@ export const convertNewTransactionToRainbowTransaction = (tx: NewTransaction): R | |||
hash: tx.hash, | |||
nonce: tx.nonce, | |||
protocol: tx.protocol, | |||
timestamp: Date.now(), |
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.
this function is used from both the "add new txn" flow and the "udpate txn" flow (for speed up / cancel)
|
||
const pendingTxCountRequest = provider.getTransactionCount(address, 'pending'); | ||
const latestTxCountRequest = provider.getTransactionCount(address, 'latest'); | ||
const [pendingTxCountFromPublicRpc, latestTxCountFromPublicRpc] = await Promise.all([pendingTxCountRequest, latestTxCountRequest]); |
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.
batched provider will batch requests sent on the provider in the same event loop in ethers v5
ba7b920
to
d1da797
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.
all fixes look good, QA Passed 👍🏽
setNonce({ | ||
address, | ||
chainId, | ||
currentNonce: currentProviderNonce > currentNonceForChainId ? currentProviderNonce : currentNonceForChainId, |
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.
this processNonces
function is no longer needed - we don't use latestConfirmedNonce
and the currentNonce
will have been made up to date already when adding / updating the pending txn
}); | ||
const localNonceData = getNonce({ address, chainId }); | ||
const localNonce = localNonceData?.currentNonce || 0; | ||
if (transaction.nonce > localNonce) { |
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.
now that we can pick up "gapped" nonces, we should check that the txn we are adding / updating has a larger nonce than what already exists before updating via setNonce
7064bfe
to
c53d8a5
Compare
c53d8a5
to
8954bc7
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.
code looks great, one v small suggestion 🥳
8954bc7
to
92ce733
Compare
92ce733
to
e24d8c8
Compare
… updating pending txns
…new txns that are using a gapped nonce
…ddNewtx and updateTxn have been consolidated and a gapped nonce can be picked up
e24d8c8
to
a0d45f8
Compare
… clobbers over data during for loop
Fixes APP-2048
Fixes APP-2040
What changed (plus any additional context for devs)
timestamp
param on new txns to keep it up to date when a txn is submitted or re-submitted (speed up/ cancel)pending
andlatest
txCount requests togetherScreen recordings / screenshots
ScreenRecording_12-06-2024.22-53-04_1.MP4
What to test