Skip to content

Conversation

@ckeshava
Copy link
Contributor

@ckeshava ckeshava commented Oct 23, 2025

High Level Overview of Change

This PR addresses two issues:

  1. Fixes Fix the chains logic in connection-manager #391
  2. Partial fix: Revoking older manifests causes DB deadlock #393 (This PR introduces an exponential DB transaction retry mechanism)

Context of Change

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

Before / After

Test Plan

Appropriate unit tests have been added in this PR. The new code changes have also been deployed in the VHS staging environment since Oct 23, 2025 4.02 PM. It will be monitored for performance changes.

@ckeshava ckeshava marked this pull request as ready for review November 4, 2025 18:24
@ckeshava
Copy link
Contributor Author

ckeshava commented Nov 4, 2025

Note to reviewers: I am working on fixing the linter errors.

*/
async function updateUNLManifestNetwork(network: string): Promise<void> {
export async function updateUNLManifestNetwork(network: string): Promise<void> {
// TODO: In an ideal scenario, the networks table should not contain undefined network.id rows
Copy link
Collaborator

Choose a reason for hiding this comment

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

network.id will never be undefined since it's the primary key, so insert would fail if id is not set

@pdp2121
Copy link
Collaborator

pdp2121 commented Nov 12, 2025

Do we need to address the first issue since the networkID PR will handle it anyway

if(validator_keys.master_key == 'nHU4bLE3EmSqNwfL4AP1UZeTNPrSPPP6FXLKXo2uqfHuvBQxDVKd' || validator_keys.signing_key == 'n9LbM9S5jeGopF5J1vBDoGxzV6rNS8K1T5DzhNynkFLqR9N2fywX') {
console.log('DEBUG: XRPL Mainnet received the following ledgers: ', Array.from(ledgerHashIndexMap))
console.log('DEBUG: Number of Validations received by the Ripple validator: ', validations.size)
console.log('DEBUG: Validations received by the Ripple validator: <ignore the second value, which is the timestamp>', validations)
Copy link

Choose a reason for hiding this comment

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

Logger has 4 different logging level

interface Logger {
  info: (arg: string) => void
  warn: (arg: string) => void
  error: (arg: string, err?: unknown) => void
  debug: (arg: string) => void
}

This comment applies to all log statements added in this PR.

incomplete: boolean,
): Promise<void> {
// obtain ledger_hashes validated by the network, strip out the ledger_index info for agreement calculation purposes
let ledgers = new Set<string>()
Copy link

@kuan121 kuan121 Nov 13, 2025

Choose a reason for hiding this comment

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

Can you fix all the alerts and the failed build?

@ckeshava
Copy link
Contributor Author

Do we need to address the first issue since the networkID PR will handle it anyway

Hello, you are correct. Point-1 is addressed in a more elegant way by a differnet PR #396 .

@ckeshava
Copy link
Contributor Author

This PR will be superceded by #396 . The attached PR is a much wider refactor of the chains logic. I have ensured that ledgers are not lost even if > 20 ledgers are missing from a chain.

Partial fix: #393 (This PR introduces an exponential DB transaction retry mechanism)

I will create another PR to rectify the victim-transaction-retry mechanism in case of db deadlock. ^^

@reviewers: thanks for your consideration.

@ckeshava ckeshava closed this Nov 19, 2025
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.

Revoking older manifests causes DB deadlock Fix the chains logic in connection-manager

3 participants