-
Notifications
You must be signed in to change notification settings - Fork 7
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 amendments status backtracking #223
Conversation
…ipple/validator-history-service into add-amendment-status-backtrack
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.
Left optional comments and approved
void createConnections() | ||
}, CM_INTERVAL) | ||
|
||
setInterval(() => { | ||
void backtrackAmendmentStatus() |
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.
Technically speaking you need to have the ability to cancel a previous request in the event this request isn’t finished by the next interval tick (even if unlikely, it could happen)
url: string, | ||
): Promise<void> { | ||
try { | ||
log.info(`Backtracking to update amendment status for ${network}...`) |
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.
Are the faucet url to debug
}) | ||
|
||
await handleWsMessageLedgerEnableAmendments( | ||
ledger as LedgerResponseCorrected, |
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.
Can you avoid as
?
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.
There's a missing field hash
for the BaseTransaction
in xrpl.js (which should be fixed), so I would need to cast to LedgerResponseCorrected
which I created to capture the hash. Once this is fixed in our client library then there would be no need for custom field or casting. I will drop a TODO for it.
log.info(`Finished backtracked amendment status for ${network}...`) | ||
} catch (error) { | ||
log.error( | ||
`Failed to backtrack amendment status for ${network} due to error: ${String( |
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.
It shouldn’t be necessary to wrap in String since toString is automatically called anyways
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 to avoid linting issue for backtick string
High Level Overview of Change
Currently we are using
p2p.livenet.ripple.com
to retrieve amendments enablement via ledger + 1 for mainnet. However, its connection is very inconsistent, led to missing information.We should backtrack every 30 minutes using clio for mainnet to retrieve this information in case the connections get lost. We would also backtrack major networks like testnet and devnet to ensure we have adequate data for these networks.
Type of Change
Test Plan
Ran without breaking on staging