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

fix amendments backtracking #298

wants to merge 1 commit into from

Conversation

pdp2121
Copy link
Collaborator

@pdp2121 pdp2121 commented Jan 22, 2025

High Level Overview of Change

There are 2 issues with amendment backtracking and status retrieval:

  • ledger rpc for API v2 with transactions and expand enabled does not match LedgerResponseExpanded field on xrpl.js. Use v1 for now.
  • s2 Clio often reached rate limit when backtracking amendments for mainnet. Use xrpl-cluster instead.

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Refactor (non-breaking change that only restructures code)
  • Tests (You added tests for code that already exists, or your new feature included in this PR)
  • Documentation Updates
  • Release

@@ -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

@@ -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

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

Successfully merging this pull request may close these issues.

3 participants