Fix validator id on bor_getSnapshot, bor_getSnapshotAtHash and bor_getCurrentValidators

[lucca30]

Description

The methods bor_getSnapshot, bor_getSnapshotAtHash, and bor_getCurrentValidators previously always returned the validator IDs as 0. Upon investigation, I identified two issues contributing to this behavior:

1. The way we were querying data from the ValidatorSet contract.
2. How validators were being retrieved while applying the snapshot.

Fix:

• Updated the logic for querying the ValidatorSet contract to correctly retrieve validator IDs.
• Adjusted the process for storing and applying snapshots to ensure the IDs are properly captured and preserved.

Changes

• Bugfix (non-breaking change that solves an issue)
• Hotfix (change that solves an urgent issue, and requires immediate attention)
• New feature (non-breaking change that adds functionality)
• Breaking change (change that is not backwards-compatible and/or changes current functionality)
• Changes only for a subset of nodes

Checklist

• I have added at least 2 reviewer or the whole pos-v1 team
• I have added sufficient documentation in code
• I will be resolving comments - if any - by pushing each fix in a separate commit and linking the commit hash in the comment reply
• Created a task in Jira and informed the team for implementation in Erigon client (if applicable)
• Includes RPC methods changes, and the Notion documentation has been updated

Cross repository changes

• This PR requires changes to heimdall
  • In case link the PR here:
• This PR requires changes to matic-cli
  • In case link the PR here:

Testing

• I have added unit tests
• I have added tests to CI
• I have tested this code manually on local environment
• I have tested this code manually on remote devnet using express-cli
• I have tested this code manually on mumbai/amoy
• I have created new e2e tests into express-cli

Manual tests

Runned the new rpc tests from matic-cli in local environment.

[cffls]

Looks good overall, thanks! Could you please add some unit tests (for the new functions)?

[lucca30]

Looks good overall, thanks! Could you please add some unit tests (for the new functions)?

Yeah, for sure. Unit tests included

[manav2401]

Have added some comments. While logically it's completely fine but I do see some places where it can be improved / optimised.

I see that you've tried to mimic the getBorValidators function but maybe we don't need to follow the exact steps.

Eg. it checks for first span but maybe we can do it locally instead of making a contract call. It iterates over all producers but maybe we can just query producers[span] for a specific span id.

Also, it'd be worth checking deploying this on a mainnet node (or amoy) to confirm if doesn't break any checks and to measure the time it takes.

[lucca30]

Have added some comments. While logically it's completely fine but I do see some places where it can be improved / optimised.

I see that you've tried to mimic the getBorValidators function but maybe we don't need to follow the exact steps.

Eg. it checks for first span but maybe we can do it locally instead of making a contract call. It iterates over all producers but maybe we can just query producers[span] for a specific span id.

Also, it'd be worth checking deploying this on a mainnet node (or amoy) to confirm if doesn't break any checks and to measure the time it takes.

Completely agree. I not did a big validation on performance impact but what I can say is that running o matic-cli locally it not seemed to impact since all the queries on contract happens without any network layer.