Skip to content

Update for Paseo support#187

Merged
wilwade merged 2 commits intomainfrom
paseo-update
Mar 27, 2024
Merged

Update for Paseo support#187
wilwade merged 2 commits intomainfrom
paseo-update

Conversation

@wilwade
Copy link
Copy Markdown
Contributor

@wilwade wilwade commented Mar 26, 2024

Goal

The goal of this PR is to add the Frequency Paseo Testnet support to the Graph SDK

Copy link
Copy Markdown
Collaborator

@aramikm aramikm left a comment

Choose a reason for hiding this comment

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

Looks good, some nits and a comment to fix the ci issue

"graphPublicKeySchemaId": 7,
"schemaMap": [
[
8,
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Just wanted to make sure that these schema id's are the same as what we registered on paseo for each

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

They are. Here's the output of the deploy script:

Generated schema mapping:
 {
  "0x203c6838fc78ea3660a2f298a58d859519c72a5efdc0f194abd6f0d5ce1838e0": {
    "tombstone": {
      "1.2": 1
    },
    "broadcast": {
      "1.2": 2
    },
    "reply": {
      "1.2": 3
    },
    "reaction": {
      "1.1": 4
    },
    "profile": {
      "1.2": 5
    },
    "update": {
      "1.2": 6
    },
    "public-key-key-agreement": {
      "1.2": 7
    },
    "public-follows": {
      "1.2": 8
    },
    "private-follows": {
      "1.2": 9
    },
    "private-connections": {
      "1.2": 10
    },
    "public-key-assertion-method": {
      "1.3": 11
    }
  }
}

}
]
]
],
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

On the paseo schema 10 does not have any settings , expecting signature required setting on it ?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Same with schema 9 it has setting set to 0 , I think private schemas require signatures enforcement

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

schema 8 is good it's Avro Paginated and setting=0

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think schema 7 has setting = 3 not sure if it corresponds to AppendOnly

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

These are the same settings as appear on mainnet. So if they are wrong, so is mainnet 😟

Copy link
Copy Markdown
Contributor

@saraswatpuneet saraswatpuneet Mar 27, 2024

Choose a reason for hiding this comment

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

Yeah I think they are wrong "in spirit" meaning most stateful operations everywhere is only calling "upsert_page" extrinsic and it does not check for schema permission but extrinsics with "...._with_signature" might be misleading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think its, fine but any application that enforces signature permission on schemas should know, not to use the ones in this config,

https://github.com/LibertyDSNP/frequency/blob/9b948d72751839e516791a4cb3ee216985554963/pallets/stateful-storage/src/lib.rs#L770C47-L770C64

@aramikm
Copy link
Copy Markdown
Collaborator

aramikm commented Mar 26, 2024

Might want to add a test like getGraphConfig with Rococo environment should return the graph config for paseo in node bridge

Copy link
Copy Markdown
Contributor

@saraswatpuneet saraswatpuneet left a comment

Choose a reason for hiding this comment

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

lgtm! schema related questions does not affect this PR

@wilwade wilwade merged commit 91423f6 into main Mar 27, 2024
@wilwade wilwade deleted the paseo-update branch March 27, 2024 16:58
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.

3 participants