Conversation
aramikm
left a comment
There was a problem hiding this comment.
Looks good, some nits and a comment to fix the ci issue
| "graphPublicKeySchemaId": 7, | ||
| "schemaMap": [ | ||
| [ | ||
| 8, |
There was a problem hiding this comment.
Just wanted to make sure that these schema id's are the same as what we registered on paseo for each
There was a problem hiding this comment.
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
}
}
}
java/example-graphsdk-client/src/test/kotlin/io/amplica/graphsdk/LibraryTest.kts
Outdated
Show resolved
Hide resolved
| } | ||
| ] | ||
| ] | ||
| ], |
There was a problem hiding this comment.
On the paseo schema 10 does not have any settings , expecting signature required setting on it ?
There was a problem hiding this comment.
Same with schema 9 it has setting set to 0 , I think private schemas require signatures enforcement
There was a problem hiding this comment.
schema 8 is good it's Avro Paginated and setting=0
There was a problem hiding this comment.
I think schema 7 has setting = 3 not sure if it corresponds to AppendOnly
There was a problem hiding this comment.
These are the same settings as appear on mainnet. So if they are wrong, so is mainnet 😟
There was a problem hiding this comment.
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
There was a problem hiding this comment.
I think its, fine but any application that enforces signature permission on schemas should know, not to use the ones in this config,
|
Might want to add a test like |
saraswatpuneet
left a comment
There was a problem hiding this comment.
lgtm! schema related questions does not affect this PR
Goal
The goal of this PR is to add the Frequency Paseo Testnet support to the Graph SDK