ripple / validator-history-service

Service for ingesting, aggregating, storing, and disbursing XRP Ledger validation related data.
ISC License
14 stars 7 forks source link

fix: `validators` api call mixed networks #217

Closed pdp2121 closed 4 months ago

pdp2121 commented 4 months ago

High Level Overview of Change

Currently the validators/{networks} call still uses chain matching. This led to test validators get pulled to main when there's a test validator validates on main accidentally, and vice versa.

for example: if after a leakage, a test validator has chain main and networks test, all of the mainnet validators will show up on validators/test, which is incorrect.

We should just use networks field for validators/{networks} itself to mitigate this effect, and also, networks is updated more frequently using ledger_hash, which makes it more accurate compared to chain calculation.

In the future, we should revise chain calculation and agreement score calculation since there's some minor issues there (will create a ticket for it). The inclusion of network-id on the validation stream would also be a permanent solution, but it is still WIP.

Type of Change

Test Plan

Leakage should be fixed in staging

ckniffen commented 4 months ago

Is chain calculation based on ledger index alone?

pdp2121 commented 4 months ago

Is chain calculation based on ledger index alone?

For the most part, yes, if you look here: https://github.com/ripple/validator-history-service/blob/main/src/connection-manager/chains.ts#L193-L245

Meanwhile, networks uses recent ledger_hash from ledger stream, which is the desired behavior as these ledgers have been closed.