Skip to content

fix amendments backtracking #298

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions src/connection-manager/wsHandling.ts
Original file line number Diff line number Diff line change
Expand Up @@ -264,11 +264,15 @@ async function backtrackNetworkAmendmentStatus(
index < currentLedger;
index += 256
) {
// Use api V1 instead of the default (V2) since we don't have the correct
// LedgerResponseExpanded type for V2
// TODO: change to V2 once the issue is fixed.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I recommend linking to the xrpl.js issue

Copy link
Contributor

Choose a reason for hiding this comment

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

Hello, do you remember the details of the issue pertaining to API v2 ? We are able to use the API v2 in the Ledger request for the latest validated ledger (a few lines above). I'm trying to understand why that would be a problem for other ledger indices.

The response format of the Ledger command appears to be compatible with the VHS code here

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes we dont need v1 now

const ledger: LedgerResponse = await client.request({
command: 'ledger',
transactions: true,
ledger_index: index,
expand: true,
api_version: 1,
})

await handleWsMessageLedgerEnableAmendments(
Expand Down
2 changes: 1 addition & 1 deletion src/shared/database/amendments.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ const AMENDMENT_VERSION_REGEX =
/\| \[(?<amendmentName>[a-zA-Z0-9_]+)\][^\n]+\| (?<version>v[0-9]*\.[0-9]*\.[0-9]*|TBD) *\|/u

export const NETWORKS_HOSTS = new Map([
['main', 'ws://s2.ripple.com:51233'],
['main', 'wss://xrplcluster.com'],
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why is this changed?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We often got rate limit while looping and sending ledger and tx RPC to clio (not sure if it's because of the recent issue) to backtrack amendments. We can either change this to xrpl-cluster completely or use it as a backup.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Has this been discussed with the Clio folks?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

RippledError(You are placing too much load on the server., {\"error\":\"slowDown\",\"error_code\":10,\"error_message\":\"You are placing too much load on the server.\",\"status\":\"error This happens once in a while with Clio

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Just posted my question on their channel

Copy link
Contributor

Choose a reason for hiding this comment

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

The backtrack interval is scheduled for every 30 minutes. However, new amendments are added rarely.

Can we execute the backtrackAmendmentStatus in a callback manner, only if new amendments are observed in a ledger? That should reduce the load on the Clio servers.

The non-admin feature command can be used to detect the enablement of new amendments.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm surprised that the testnet and devnet haven't thrown similar "slowdown" errors, given that the amendments' status are expected to be similar between the main, dev and test networks.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We are doing this because we are only using the entry node (so one node only) per each network to retrieve live amendments on Flag + 1 ledger. The backtracking would be just in case the entry node is down, we will use Clio. We only fetch Flag +1 ledgers in the last 30 minutes so I dont think it send too many requests to Clio

['test', 'wss://s.altnet.rippletest.net:51233'],
['dev', 'wss://s.devnet.rippletest.net:51233'],
])
Expand Down
Loading