-
Notifications
You must be signed in to change notification settings - Fork 45
solana ce first pass #12239
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
base: master
Are you sure you want to change the base?
solana ce first pass #12239
Conversation
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.
Good start
|
||
// Check if directory exists | ||
if (!existsSync(dirPath)) { | ||
console.warn(`IDL directory not found: ${dirPath}`); |
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.
Should replace all console
logs with our logger created with const log = logger(import.meta)
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.
the logger package(commonwealth core) isnt imported in evm-protocols, should we import it?
@@ -0,0 +1,149 @@ | |||
/** |
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.
If we don't expect the number of idls to grow it may be simpler to just import the JSON directly rather than having functions to read the files dynamically.
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 think given this works we could stick with this approach unless there is an issue with it - idls may or may not grow, we never know(lol) and this allows for the fastest addition with the least code changes when a new program or network is added with the upfront complexity here.
@@ -0,0 +1,28 @@ | |||
import Sequelize from 'sequelize'; |
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.
Is there a reason we create a new table instead of just renaming the other table to LastProcessedBlock
?
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.
Cursors justification was query performance per worker and table organization. I can refactor to use LastProcessedBlock if we dont want an extra table
), | ||
LOG_TRACE: process.env.SOLANA_CE_LOG_TRACE !== 'false', | ||
MAX_SLOT_RANGE: parseInt( | ||
process.env.SOLANA_CE_MAX_SLOT_RANGE ?? '100', |
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 think we can increase default value by a lot. Alchemy limits for EVM are much higher so I presume Solana is similar. Should research the exact slot limit Alchemy sets and then use 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.
Could not find alchemy docs on this but bumped to 10000
Link to Issue
Closes: #12186
Description of Changes