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

feat: cache transaction receipts #2181

Draft
wants to merge 23 commits into
base: master
Choose a base branch
from

Conversation

dewanshparashar
Copy link
Contributor

No description provided.

Copy link

vercel bot commented Jan 10, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
arbitrum-token-bridge ✅ Ready (Inspect) Visit Preview Jan 17, 2025 3:10pm

import { BigNumber } from 'ethers'

const cacheKey = `arbitrum:bridge:tx-receipts`
const cacheLogs = false
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

to be removed when ready for review

@@ -107,7 +108,8 @@ const getProviderForChainCache: {

function createProviderWithCache(chainId: ChainId) {
Copy link
Contributor Author

@dewanshparashar dewanshparashar Jan 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

getProviderForChainId's provider is passed down to almost everywhere in the code, especially in Tx history. Hence, changing this provider to EnhancedProvider should upgrade it across our codebase.


const cacheKey = `arbitrum:bridge:tx-receipts-cache`
const enableCaching = true

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

txReceiptToString(txReceipt: TransactionReceipt): string
txReceiptFromString(stringified: string): TransactionReceipt


type CachedReceipts = Record<string, TransactionReceipt>

const cacheKey = `arbitrum:bridge:tx-receipts-cache`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
const cacheKey = `arbitrum:bridge:tx-receipts-cache`
const localStorageKey = `arbitrum:bridge:tx-receipts-cache`

if (!enableCaching) return false

// Don't cache failed transactions
if (typeof txReceipt.status !== 'undefined' && txReceipt.status === 0) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (typeof txReceipt.status !== 'undefined' && txReceipt.status === 0) {
if (txReceipt.status === 0) {

// Finality checks to avoid caching re-org'ed transactions
if (
(chainId === ChainId.Ethereum && txReceipt.confirmations < 65) ||
(chainId === ChainId.Sepolia && txReceipt.confirmations < 5)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add holesky

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants