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.

Sign up for GitHub

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

Jump to bottom

add new platform user VC: daren market #2903

Merged
merged 5 commits into from
Jul 17, 2024
Merged

add new platform user VC: daren market #2903

merged 5 commits into from
Jul 17, 2024

Conversation

BillyWooo
Copy link
Collaborator

As title, add new VC support.

Related changes: litentry/vc-jsonschema#42

Testing Evidences

Please attach any relevant evidences if applicable

image

ubuntu@ovh-dev-2:~/repo/litentry-parachain-2nd/tee-worker/bin$ ./litentry-cli trusted -d link-identity did:litentry:substrate:0x8eaf04151687736326c9fea17e25fc5287613693c912909cb226aa4794f26a48 did:litentry:evm:0x49ad262c49c7aa708cc2df262ed53b64a17dd5ee ethereum

ubuntu@ovh-dev-2:~/repo/litentry-parachain-2nd/tee-worker/bin$ ./litentry-cli trusted -d request-vc did:litentry:substrate:0x8eaf04151687736326c9fea17e25fc5287613693c912909cb226aa4794f26a48 -a "platform-user daren-market"
>>> identity: Substrate(Address32)
>>> nonce: 1
>>> assertions: [PlatformUser(DarenMarket)]
----Generated VC-----
{"@context":["https://www.w3.org/2018/credentials/v1","https://w3id.org/security/suites/ed25519-2020/v1"],"id":"0x836324c4e705de41329e69da5d3ab32d59fa37639f4c7167349d58820f9bdf6e","type":["VerifiableCredential"],"credentialSubject":{"id":"did:litentry:substrate:0x8eaf04151687736326c9fea17e25fc5287613693c912909cb226aa4794f26a48","description":"You are a user of a certain platform","type":"Platform user","assertionText":"","assertions":[{"and":[{"src":"$platform","op":"==","dst":"DarenMarket"}]}],"values":[true],"endpoint":"http://localhost:9933"},"issuer":{"id":"did:litentry:substrate:0x1f56dd77aa115346161ab6c593743dab5e86b821b7cf0b8c384ed0fc21b1cf94","name":"Litentry TEE Worker","mrenclave":"8ryMqWvG6UtYrr4hTcVhj7himRkj6atovRDzVcTGZaam","runtimeVersion":{"parachain":9183,"sidechain":109}},"issuanceDate":"2024-07-16T15:39:15.636602818+00:00","parachainBlockNumber":13,"sidechainBlockNumber":16,"proof":{"created":"2024-07-16T15:39:15.636771026+00:00","type":"Ed25519Signature2020","proofPurpose":"assertionMethod","proofValue":"93ab192b43318b473f87584e8b7b628db78b8ddfc9fb528284f446fd40aec55a410e5cff8181b5d30654701ad1f7e993f460a42a4be6bcf4c8ad5b2be767760e","verificationMethod":"0x1f97141f5c3d41d5be0ffbd127b36fb37096a8b05f9f07b8a5515bf19c7ac3a9"},"credentialSchema":{"id":"https://raw.githubusercontent.com/litentry/vc-jsonschema/main/dist/schemas/24-platform-user/1-1-1.json","type":"JsonSchemaValidator2018"}}
ubuntu@ovh-dev-2:~/repo/litentry-parachain-2nd/tee-worker/bin$

@BillyWooo BillyWooo added the C1-noteworthy Non-breaking change but is worth noticing for client label Jul 16, 2024
@linear Linear
Copy link

linear bot commented Jul 16, 2024

P-932 Daren Platform User VC

@BillyWooo BillyWooo force-pushed the p-932-daren-platform-user-vc branch from cc23492 to f2050e9 Compare July 16, 2024 15:55
@Kailai-Wang Kailai-Wang requested a review from a team July 16, 2024 20:19
Kailai-Wang
Kailai-Wang reviewed Jul 16, 2024
View reviewed changes
Copy link
Collaborator

@Kailai-Wang Kailai-Wang left a comment

Choose a reason for hiding this comment

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

Thanks!

Apart from this PR itself, we need to double-check:

  • the API keys are listed and imported in the deployment cc @m1iktea
  • if the enum change is a breaking change cc @jonalvarezz

bitacross-worker/docker/docker-compose.yml Outdated
Copy link
Collaborator

Choose a reason for hiding this comment

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

We don't need to change it - actually we should remove all data provider envs for bc-worker

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

sure. Let's do it in another PR to remove all unrelated entries.
P-935 : remove env entries

primitives/core/src/assertion/platform_user.rs
Comment on lines +28 to +30
MagicCraftStaking,
#[codec(index = 2)]
DarenMarket,
Copy link
Collaborator

Choose a reason for hiding this comment

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

I assume this will be a breaking change for the SDK? @jonalvarezz

tee-worker/litentry/core/assertion-build-v2/src/platform_user/mod.rs

let mut config = DataProviderConfig::new().unwrap();
config.set_karat_dao_api_url(url).unwrap();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hmm so we never set magic_craft? How did it work before? 🤔

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Previously magic_craft was not tested.

tee-worker/litentry/core/service/src/platform_user/daren_market_user.rs
Comment on lines +43 to +53
|address| match client.user_verification(address.to_string(), true) {
Ok(response) =>
if response.created {
result = true;
Ok(LoopControls::Break)
} else {
Ok(LoopControls::Continue)
},
Err(err) => Err(err.into_error_detail()),
},
AbortStrategy::ContinueUntilEnd::<fn(&_) -> bool>,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hmm fail_fast is true here but we are using AbortStrategy::ContinueUntilEnd, it seems a bit confusing.

Or maybe they shouldn't be used together?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fail_fast is used for querying data from data provider. AbortStrategy::ContinueUntilEnd is for the loop through all addresses. So they are not conflicting with each other.

@BillyWooo
Copy link
Collaborator Author

@Kailai-Wang regarding the API KEY, our partner has not yet ready to provide it. We are still waiting for them to test it out. CC: @m1iktea

higherordertech
higherordertech approved these changes Jul 17, 2024
View reviewed changes
Copy link
Contributor

@higherordertech higherordertech left a comment

Choose a reason for hiding this comment

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

Approved on condition that negative case i.e. false value VC is also tested

@BillyWooo
Copy link
Collaborator Author

Confirmed with partner Daren. They will provide public API. So there will be no API KEY.

jonalvarezz
jonalvarezz reviewed Jul 17, 2024
View reviewed changes
tee-worker/client-api/parachain-api/prepare-build/interfaces/vc/definitions.ts
Comment on lines -245 to 246
_enum: ["KaratDaoUser", "MagicCraftStakingUser"],
_enum: ["KaratDao", "MagicCraftStaking", "DarenMarket"],
},
Copy link
Contributor

Choose a reason for hiding this comment

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

we will need to update client-api, but not a breaking change for client-sdk nor clients in general.

@BillyWooo BillyWooo merged commit 31bcf9b into dev Jul 17, 2024
32 checks passed
@BillyWooo BillyWooo deleted the p-932-daren-platform-user-vc branch July 17, 2024 17:05
@jonalvarezz jonalvarezz mentioned this pull request Jul 17, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Kailai-Wang Kailai-Wang Kailai-Wang left review comments

@jonalvarezz jonalvarezz jonalvarezz left review comments

@higherordertech higherordertech higherordertech approved these changes

Assignees
No one assigned
Labels
C1-noteworthy Non-breaking change but is worth noticing for client
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@BillyWooo @jonalvarezz @Kailai-Wang @higherordertech