-
Notifications
You must be signed in to change notification settings - Fork 51
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: Add redis caching for account factory address #639
base: main
Are you sure you want to change the base?
Conversation
const cachedFactoryAddress = await redis.get( | ||
`account-factory:${accountAddress.toLowerCase()}`, | ||
); | ||
accountFactoryAddress = getAddress(cachedFactoryAddress ?? ""); |
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 is indirect to what you want:
- if cachedFactoryAddress exists, set accountFactoryAddress to the checksum version of it
- else accountFactoryAddress is undefined or null
Write the code that way. The current way has some unexpected behavior (what does getAddress("")
do? You have to read the code to find out):
let accountFactoryAddress: Address | undefined;
try {
const cached = await redis.get(
`account-factory:${accountAddress.toLowerCase()}`,
);
if (cached) {
accountFactoryAddress = getAddress(cached);
}
} catch {}
}); | ||
|
||
if (!accountFactoryAddress) { | ||
const onchainFactoryResult = (await readContract({ |
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.
Unless I'm missing something, shouldn't you just set the response directly to accountFactoryAddress
instead of declaring a new var?
method: "function factory() view returns (address)", | ||
params: [], | ||
})) as Address; | ||
accountFactoryAddress = onchainFactoryResult; |
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.
You're missing a line here to set the cache after retrieving it.
This PR is stale because it has been open for 7 days with no activity. Remove stale label or comment or this PR will be closed in 3 days. |
PR-Codex overview
This PR focuses on enhancing the
generateSignedUserOperation
function inuserOperation.ts
by introducing a caching mechanism to retrieve the Factory Contract Address more efficiently.Detailed summary
getAddress
function