Update Sdk with Arbitrum support #4720
Conversation
albert-llimos
changed the title
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #4720 +/- ##
======================================
- Coverage 72% 72% -0%
======================================
Files 417 418 +1
Lines 69531 69445 -86
Branches 69531 69445 -86
======================================
- Hits 50245 50077 -168
- Misses 16879 16960 +81
- Partials 2407 2408 +1 ☔ View full report in Codecov by Sentry. |
albert-llimos
changed the title
| await approveTokenVault( | ||
| sourceAsset, | ||
| ( | ||
| BigInt(amountToFineAmount(defaultAssetAmounts(sourceAsset), assetDecimals(sourceAsset))) * |
There was a problem hiding this comment.
we should pull this out into a function that takes a source asset and amount. We use this pattern of BigInt(amountToFineAmount... in a number of other places too
| gasSwapScheduledHandle = observeSwapScheduled( | ||
| sourceAsset, | ||
| destChain === 'Ethereum' ? Assets.Eth : ('ArbEth' as Asset), | ||
| destChain === 'Ethereum' ? Assets.Eth : Assets.ArbEth, |
There was a problem hiding this comment.
just noticed there are lot of other places we still use asset as string, we should move to this Assets type. (ofc, would be another ticket/PR, not directly relevant)
There was a problem hiding this comment.
yes, there is definitely some cleanup to do in multiple places. I'll aim to get it done when I get some time.
…-validator * origin/main: fix: broker flakiness on bouncer CI (#4736) fix: migration for earned fees (#4733) feat: handle prewitness deposits (#4698) Update Sdk with Arbitrum support (#4720) chore: remove insta deprecated warning (#4734) refactor: move vanity names to account roles pallet (#4719) Arbitrum backup rpc (#4730) # Conflicts: # state-chain/pallets/cf-validator/src/lib.rs # state-chain/pallets/cf-validator/src/mock.rs # state-chain/pallets/cf-validator/src/tests.rs
Checklist
Please conduct a thorough self-review before opening the PR.
Summary
Updating SDK with the latest version that has Arbitrum support.
After doing that I found a problem with nonces. This is my hypothesis which turned out to be true:
"The error is nonce too high. I have checcked the nonce counter logic in the bouncer and it looks good, also the prints look like we are not skipping anything. Looks solid. It even seems to happen when only running all the contract swaps via the SDK in parallel, but I'm pretty sure there is no change on the SDK either on that front.
I believe what is happening is that we have a race condition when running all swaps (normal deposit swaps & contract swaps via SDK). They are all using the same nonces and if some transaction lands in the wrong order the node will reject the transaction, even though it would eventually get mined when the transaction prior to it eventually lands.
My best guess is that we are starting to see this now because the amount of parallel swaps has increased substantially with the addition of USDT and Arbitrum assets."
For contract swaps I have modifed the logic to start each swap from a different account so there can't be issues with nonces. That works.