Skip to content
This repository has been archived by the owner on May 22, 2023. It is now read-only.

Staker rewards script - SLA part #625

Merged
merged 62 commits into from Dec 1, 2020
Merged

Staker rewards script - SLA part #625

merged 62 commits into from Dec 1, 2020

Conversation

lukasz-zimnoch
Copy link
Member

@lukasz-zimnoch lukasz-zimnoch commented Nov 24, 2020

Refs: #602

Here we introduce the staker rewards script. This PR aims to introduce:

  • The structure of the script
  • Implementation of the data cache and helpers
  • Computations of the SLA part

The script can be invoked by running the following command:

ETH_HOSTNAME=<eth-ws-hostname> \
node --experimental-json-modules rewards.js \
<interval-start-unix-timestamp> <interval-end-unix-timestamp>

Once invoked, the script validates the passed interval, refreshes the cache, and performs computations of the SLA for each operator. An example output looks as follows:

sla-output

Specific parameters for each operator are calculated based on keeps the operator is a member of, in the following way:

  • keygenCount: it's the number of keeps whose creation timestamp is within the given interval
  • keygenFailCount: it's the part of keeps from the keygenCount number which have been eventually terminated due to a keygen fail
  • keygenSLA: it's a parameter calculated as 100 - ((keygenFailCount * 100) / keygenCount and rounded to the floor
  • signatureCount: it's the number of keeps which changed their statuses from active to closed or from active to terminated due to signature fail, within the given interval AND have been asked for signing at least once.
  • signatureFailCount: it's the part of keeps from the signatureCount number which changed their statuses from active to terminated due to signature fail
  • signatureSLA: it's a parameter calculated as 100 - ((signatureFailCount * 100) / signatureCount and rounded to the floor

@lukasz-zimnoch lukasz-zimnoch changed the title Staking rewards inspector Staker rewards inspector Nov 24, 2020
@lukasz-zimnoch lukasz-zimnoch changed the title Staker rewards inspector Staker rewards inspector - SLA part Nov 26, 2020
staker-rewards/inspector/scripts/inspect-staker-rewards.js Outdated Show resolved Hide resolved
staker-rewards/inspector/README.adoc Outdated Show resolved Hide resolved
staker-rewards/inspector/README.adoc Outdated Show resolved Hide resolved
staker-rewards/inspector/scripts/lib/cache.js Outdated Show resolved Hide resolved
staker-rewards/inspector/scripts/lib/context.js Outdated Show resolved Hide resolved
staker-rewards/inspector/scripts/lib/cache.js Outdated Show resolved Hide resolved
staker-rewards/inspector/scripts/lib/cache.js Outdated Show resolved Hide resolved
staker-rewards/inspector/scripts/lib/cache.js Outdated Show resolved Hide resolved
staker-rewards/inspector/scripts/lib/contract-helper.js Outdated Show resolved Hide resolved
staker-rewards/inspector/scripts/lib/cache.js Outdated Show resolved Hide resolved
@lukasz-zimnoch lukasz-zimnoch changed the title Staker rewards inspector - SLA part Staker rewards script - SLA part Nov 27, 2020
@lukasz-zimnoch lukasz-zimnoch marked this pull request as ready for review November 27, 2020 09:20
Changed the logic which builds the
`deactivatedKeeps` array to not include
liquidations in the signature SLA as they
distort this metric.
Added an additional keep to the test cache
which doesn't contain the tested operator to
add some diversity to the test set.
@@ -0,0 +1,22 @@
{
Copy link
Member

Choose a reason for hiding this comment

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

As we initiate a new NPM package in staker-rewards directory can we also configure JS linting? Defining this now will be super handy for following PRs.

// due to one of the following causes:
// - keep has been closed after delivering a signature successfully
// - keep has been terminated after not delivering a signature
const deactivatedKeeps = [].concat(closedKeeps, signatureFailKeeps)
Copy link
Member

Choose a reason for hiding this comment

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

There is one edge case we missed. Keep can be closed by the application even though it was never asked to provide a signature. In the case of tBTC, this is a funding timeout scenario. For example, if we generate mainnet report from the moment of deploying contracts and look at the operator 0xF1De9490Bf7298b5F350cE74332Ad7cf8d5cB181, we see this operator has signatureCount = 159:

image

It does not seem right as a significant number of closedKeeps are funding time-out keeps that were never asked for a signature.

I think the fix should be to add a function to keep.js that will let us say if the given keep was ever asked to provide a signature (one occurrence of SignatureRequested is enough) and if not, filter out such keeps. For example:

const deactivatedAndAskedForSigning = deactivatedKeeps.filter(keep => wasAskedForSignature(keep))

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok, done. Unfortunately, this change increases the script execution time significantly. For now, I don't see an easy way to optimize those checks.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, agreed. We have to live with it for now. In the future, we could be caching if a keep was asked for a signature refreshing all cases when the keep wasn't asked. Probably, adding some optimization with interval start/end date and whether there was a chance the signature was requested (e.g. it is not a case for closed keeps before interval start date).

Let's leave this discussion open for now - maybe we can implement some optimization in a separate PR at the very end, once we'll have all other pieces working together.

Added the `wasAskedForSignature` filter which
adds a new filtering layer checking whether
a deactivated keep has been requested for
signing.
@pdyraga pdyraga merged commit ead9e41 into master Dec 1, 2020
@pdyraga pdyraga deleted the staker-rewards-inspector branch December 1, 2020 15:57
@pdyraga pdyraga added this to the solidity/v1.4.0 milestone Dec 3, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants