Skip to content
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

Add requirements script for node operator transparency and monitoring #90

Closed
wants to merge 22 commits into from

Conversation

randyramig
Copy link

This PR adds the ability for a node operator to easily check their current tbtc rewards requirements status based on the Source of Truth data used for rewards calculations.

The motivation is to improve transparency for node operators and provide an ability to monitor their nodes and act accordingly when a node is in danger of not meeting requirements. Nobody wants to get to the end of the month only to find that they have not met requirements.

See src/scripts/tbtcv2-rewards/MONITORING.md for details and instructions.

Summary of changes:

  • Created requirements.ts|sh for usage more appropriate to monitoring. This involved copying over rewards.ts|sh, removing unneeded code that only applied to monthly rewards, and using operator address instead of staker address. Use of staker address is not very performant for the monitoring use case because you have to loop through every operator address in the bootstrap data and make eth contract calls to resolve.

  • Created staker2operator.ts|sh to make it easy to obtain operator address from staker address

  • An important aspect of this PR is the refactoring of common code for both rewards.ts and requirements.ts to use. I did this so that changes in code that is used for rewards.ts also applies to requirements.ts, we don't want requirements monitoring to be forked from rewards code. I simply moved shared code to Utils as a low-cost way to do the refactor.

I verified that rewards.sh|ts are working, but this should be double-checked for merging this PR.

Copy link

@dimpar dimpar left a comment

Choose a reason for hiding this comment

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

A couple of comments mainly to sync with the latest code from the official main branch since there were some changes recently.

I checked only rewards.ts and utils.ts and focused around rewards requirements to make sure this is the same as in the official repo. All other code and refactoring bits leaving in @manumonti and @lukasz-zimnoch hands.

offset,
endRewardsTimestamp,
requiredUptime,
october17Timestamp,
Copy link

Choose a reason for hiding this comment

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

Note that all the code handling october 17 rewards case was removed. Please update/copy from the official repo for the latest changes. See 055f849

const latestClientTagTimestamp = Number(latestClient[1])
const secondToLatestClient = clientReleases[1].split("_")
const secondToLatestClientTag = secondToLatestClient[0]

Copy link

Choose a reason for hiding this comment

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

Please update the code with the latest changes from main. It should also handle the 3rd release case now. See https://github.com/threshold-network/merkle-distribution/blob/main/src/scripts/tbtcv2-rewards/rewards.ts#L324 as one of the changes that was added recently.


let isVersionSatisfied = true
const upgradeCutoffDate = latestClientTagTimestamp + ALLOWED_UPGRADE_DELAY
if (upgradeCutoffDate < startRewardsTimestamp) {
Copy link

Choose a reason for hiding this comment

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

Please c&p the latest code around versioning validation from main. We modified some code recently.

@manumonti
Copy link
Member

This work was redone in #125

@manumonti manumonti closed this Feb 29, 2024
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.

None yet

3 participants