Modify PRE operator schema's entity #11

manumonti · 2022-05-27T12:30:55Z

Note: This PR must be merged after graphprotocol/graph-tooling#1108.

Schema's changes:

Entity name changed to PREOperator, which is more descriptive.
Added OperatorBonded event handler, so PREOperator instance is created when a new operator is bonded.
Added confirmationTimestamp field, which will store the time in which the operator is confirmed.

Other changes:

Update getEpochStakeId function. This function from utils returns a string with the Epoch Stake's ID. Now it accepts an epoch ID. If this is not passed, it takes the current epoch count. Also, the staking provider argument type have changed from String to Address for simplicity.

A stake can have different stake types at the same time (T, NuInTStake and KeepInTStake), so the StakeType's field can be misleading.

- Entity name changed to PREOperator, which is more descriptive. - Added OperatorBonded event handler, so PREOperator instance is created when a new operator is bonded. - Added confirmationTimestamp field, which will store the time in which the operator is confirmed. - Update getEpochStakeId function to accept epoch ID. If no argument passed, the function takes the current epoch count.

r-czajkowski · 2022-06-14T09:08:18Z

schema.graphql

@@ -78,10 +78,11 @@ type TokenholderDelegation implements Delegation @entity {
  delegators: [Account!] @derivedFrom(field: "delegatee")
 }

-type ConfirmedOperator @entity {
+type PREOperator @entity {
+  # ID is the staking provider address
  id: ID!


I think ID should be the operator address, not the staking provider because we are in the PREOperator entity and we should add a relationship between PREOperator and StakeData entity.

That's a good point. I agree with ID being the operator's address.

But I don't see clearly how to add a relationship between PREOperator and StakeData. Do you mean a Reverse Lookup? This will imply to add a new field in StakeData (probably called preoperator), and in my opinion, we shouldn't mix Threshold Network Staking (StakeData) with an Threshold application of the network (PRE).

Or maybe you mean to use One-To-Many relationship? In that case, do you agree with this schema?

type PREOperator @entity { # ID is the operator address id: ID! confirmationTimestamp: BigInt stakes: [StakeData!]! }

The PREOperator entity will include an array of all stakeDatas that use this operator.

What do you think?

Yep makes sense. But it should probably be a one-to-one relationship because of this requirement https://github.com/nucypher/nucypher-contracts/blob/main/contracts/contracts/SimplePREApplication.sol#L193 cc @cygnusv.

Another solution could be a schema like this:

type StakeData @entity { ... applications: [Application!] @derivedFrom(field: "stake") } interface Application { id: ID! stake: StakeData! authorizedStakeAmount: BigInt! } type PREApplication implements Application @entity { id: ID! // id = "operatorAddress-preapplication" stake: StakeData! authorizedStakeAmount: BigInt! confirmationTimestamp: BigInt! ... // other fields related to the pre app } // other applications in the future eg. RandomBeaconApplication type RandomBeaconApplication implement Application @entity { ... }

So in that case we are ready for multi-staking app authorization and the applications field makes sense in the StakeData entity because we authorize applications to use a given stake. What do you think? @manumonti @cygnusv ofc we can address it in a separate PR just a thought.

I find your proposal very reasonable and I mostly agree with that.

Only one comment about this:

type PREApplication implements Application @entity { id: ID! // id = "operatorAddress-preapplication" [...]

I think it could be easier to implement this if id is stakingProviderAdress-preapplication. I understand that this entity is PREApplication so ID should include operator address, but there is a case which makes hard to implement this:

Check this line: https://github.com/nucypher/nucypher-contracts/blob/710b68713b5b8fece0d0b1c5de376dba1d7d68e7/contracts/contracts/SimplePREApplication.sol#L206

If I am not wrong, this means that when an operator is unbonded, an operatorBonded event is emitted with these parameters:

stakingProvider = staking provider address

operator = 0x00000000000000000

(please @cygnusv , @vzotova can you confirm?)

So, if the ID is composed of the operator address, we can't easily find the PREOperator entity in order to modify or delete it when this type of event is received.

What do you think, @r-czajkowski ?

Yeah, that's right and it works for me. I'm curious about David's opinion.

Unbonding operator will lead to operator = 0x00000000000000000 in an event

So, if the ID is composed of the operator address, we can't easily find the PREOperator entity in order to modify or delete it when this type of event is received.

On the other hand, this is probably a rare scenario (but I'm not sure) and there will be 3 apps (at least for now: tbtc, random beacon, and pre) so I don't mind iterating through applications from the StakeData entity and finding the app which contains -preapplication in the ID and then removing it from the store. Just a thought. Both solutions work for me 👍.

Hi @r-czajkowski. Although I think your proposal scheme fits for Threshold Applications (PREApplication, RandomBeacon, TBTC...), IMHO it can't be applied to SimplePREApplication because:

It doesn't make sense to have an authorizedStakeAmount field in SimplePREApplication since events for authorized stake are not emitted yet.

There is a discussion in progress about these events: threshold-network/solidity-contracts#88 (comment).

I have thought about calling the authorizedStake function from SimplePREApplication contract (https://github.com/nucypher/nucypher-contracts/blob/710b68713b5b8fece0d0b1c5de376dba1d7d68e7/contracts/contracts/SimplePREApplication.sol#L95-L101), but the state of this field won't be synced since no new events are emitted if stake amounts are modified.

In any case, about this:

On the other hand, this is probably a rare scenario (but I'm not sure) and there will be 3 apps (at least for now: tbtc, random beacon, and pre) so I don't mind iterating through applications from the StakeData entity and finding the app which contains -preapplication in the ID and then removing it from the store.

As I understand, this can't work in your proposal schema since the applications field is a derived field. And it is not possible to fetch derived fields in the mappings.ts, since these are built at query time. See https://github.com/graphprotocol/graph-ts/issues/219.

In my opinion, we should create a new issue or PR with your proposed schema and address it when a definitive version of some Threshold application (PREApplication, TBTC, RandomBeacon) is deployed.

Please, let me know your opinion about this.

I finally created a issue addressing this. So if @r-czajkowski agree, we can continue the conversation there.

#23

r-czajkowski · 2022-06-17T07:41:33Z

src/mapping.ts

+  const timestamp = event.params.startTimestamp
+
+  const preApplication = getOrCreatePreApplication(stakingProvider)
+  if (operator == Address.zero()) {


It is always safer to use === instead of ==, unless there is a really good reason to use ==.

Done! thank you for the tip

r-czajkowski · 2022-06-17T07:44:32Z

subgraph.yaml

@@ -71,11 +71,13 @@ dataSources:
      apiVersion: 0.0.5
      language: wasm/assemblyscript
      entities:
-        - Operator
+        - PREOperator


We renamed the entity to SimplePREApplication. Let's update here as well.

Good catch! Done.

The contract used for PRE application is 'SimplePREApplication' instead of 'PREApplication' since the current version of the app is not the definitive one. Also, this GraphQL entity includes a field for 'StakeData', and two timestamps fields, one for when the operator is bonded and another for when the operator is confirmed.

r-czajkowski

manumonti requested review from cygnusv and r-czajkowski May 27, 2022 12:30

manumonti mentioned this pull request May 27, 2022

Split mapping main file #12

Closed

manumonti marked this pull request as draft June 7, 2022 16:41

manumonti force-pushed the schema-cleanup-2 branch from e22be4b to eeeac19 Compare June 8, 2022 07:12

manumonti force-pushed the operator-events branch from a79ad0b to c2834fa Compare June 8, 2022 07:47

manumonti marked this pull request as ready for review June 8, 2022 07:49

manumonti mentioned this pull request Jun 8, 2022

Split mappings file according to contract event emitter #16

Merged

manumonti force-pushed the schema-cleanup-2 branch from eeeac19 to 0ef1044 Compare June 10, 2022 11:41

manumonti force-pushed the operator-events branch from c2834fa to ae02310 Compare June 10, 2022 11:45

manumonti force-pushed the schema-cleanup-2 branch from 0ef1044 to 44c51a1 Compare June 10, 2022 12:18

manumonti force-pushed the operator-events branch from ae02310 to a0ae207 Compare June 10, 2022 12:27

manumonti force-pushed the schema-cleanup-2 branch from 44c51a1 to 1600fac Compare June 10, 2022 12:42

manumonti added 2 commits June 10, 2022 14:43

Delete schema's stakeType field from StakeData

32ae77d

A stake can have different stake types at the same time (T, NuInTStake and KeepInTStake), so the StakeType's field can be misleading.

manumonti force-pushed the operator-events branch from a0ae207 to e4a63ea Compare June 10, 2022 12:43

r-czajkowski requested changes Jun 14, 2022

View reviewed changes

Base automatically changed from schema-cleanup-2 to main June 15, 2022 12:39

manumonti mentioned this pull request Jun 16, 2022

Add applications field to stakeData entity #23

Closed

manumonti requested a review from r-czajkowski June 16, 2022 18:33

r-czajkowski requested changes Jun 17, 2022

View reviewed changes

manumonti force-pushed the operator-events branch from 2aaf504 to 88380ab Compare June 17, 2022 08:24

r-czajkowski approved these changes Jun 17, 2022

View reviewed changes

manumonti merged commit 73a83c9 into main Jun 17, 2022

manumonti deleted the operator-events branch June 17, 2022 08:55

manumonti mentioned this pull request Jun 17, 2022

Add Old Operator parameter to OperatorBonded event vzotova/nucypher#7

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Modify PRE operator schema's entity #11

Modify PRE operator schema's entity #11

manumonti commented May 27, 2022

r-czajkowski Jun 14, 2022

manumonti Jun 14, 2022 •

edited

Loading

r-czajkowski Jun 15, 2022

manumonti Jun 15, 2022 •

edited

Loading

r-czajkowski Jun 15, 2022

vzotova Jun 15, 2022

r-czajkowski Jun 15, 2022 •

edited

Loading

manumonti Jun 16, 2022 •

edited

Loading

manumonti Jun 16, 2022

r-czajkowski Jun 17, 2022

manumonti Jun 17, 2022

r-czajkowski Jun 17, 2022

manumonti Jun 17, 2022

r-czajkowski left a comment

Modify PRE operator schema's entity #11

Modify PRE operator schema's entity #11

Conversation

manumonti commented May 27, 2022

Choose a reason for hiding this comment

manumonti Jun 14, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

manumonti Jun 15, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

r-czajkowski Jun 15, 2022 • edited Loading

Choose a reason for hiding this comment

manumonti Jun 16, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

r-czajkowski left a comment

Choose a reason for hiding this comment

manumonti Jun 14, 2022 •

edited

Loading

manumonti Jun 15, 2022 •

edited

Loading

r-czajkowski Jun 15, 2022 •

edited

Loading

manumonti Jun 16, 2022 •

edited

Loading