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.

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

Feat/add proof soulbound nft #2

Open
wants to merge 14 commits into
base: feat/refactor
Choose a base branch
from

Conversation

JohnGuilding
Copy link

@JohnGuilding JohnGuilding commented Nov 22, 2024

Adds soulbound NFT representing a valid ZK Email proof

  • Soulbound
  • Recipient should be committed to in the proof
  • Metadata stored:
struct ZKEmailProofMetadata {
    uint256 blueprintId;
    Proof proof;
    uint256[] publicOutputs;
    string decodedPublicOutputs;
}
  • Basic tests for tokenURI, safeMint and safeTransferFrom
  • Adds basic access control
  • Decoded public outputs should be flattened json. It is currently expected that this is formatted correctly before minting

) public {
// Owner should be committed to in each proof. This prevents
// frontrunning safeMint with a valid proof but malicious "to" address
if (address(uint160(proof[0])) != to) {
Copy link
Author

Choose a reason for hiding this comment

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

Wasn't sure on correct format here @Bisht13 - would the address be in the proof like this?

Copy link
Member

Choose a reason for hiding this comment

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

this would be publicOutputs[1]

uint256 blueprintId;
uint256[] proof;
address verifier;
string publicOutputs;
Copy link
Author

Choose a reason for hiding this comment

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

Wasn't sure on correct format here and ran out of time. Perhaps this would be an array of key value strings or similar and then we could add them to the metadata in the same way that proofJson is added in tokenURI()

Copy link
Member

Choose a reason for hiding this comment

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

proof is going to be [uint[2], uint[2][2], uint[2]] always, publicOutputs is going to be uint256[], then we also need a decoded public output field which would be included in the token uri

Copy link
Author

Choose a reason for hiding this comment

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

TODO notes

  1. Use [uint[2], uint[2][2], uint[2]] for proof
  2. Loop over public (uint256[]) outputs the same way the proof currently works
  3. Add decoded public outputs which should be flattened json

@JohnGuilding JohnGuilding marked this pull request as ready for review November 22, 2024 17:45
@JohnGuilding
Copy link
Author

Didn't have time to add a fleshed out test with a detailed flattened json. But @Bisht13 was envisioning something like this - wdyt?

unflattened:

{
  "user": {
    "id": 1,
    "name": "John Smith",
    "contact": {
      "email": "[email protected]",
      "phone": "07123 456789" 
    }
  }
}

flattened:

{
  "user.id": 1,
  "user.name": "John Smith",
  "user.contact.email": "[email protected]"
  "user.contact.phone": "07123 456789"
}

@Bisht13
Copy link
Member

Bisht13 commented Nov 27, 2024

Didn't have time to add a fleshed out test with a detailed flattened json. But @Bisht13 was envisioning something like this - wdyt?

unflattened:

{
  "user": {
    "id": 1,
    "name": "John Smith",
    "contact": {
      "email": "[email protected]",
      "phone": "07123 456789" 
    }
  }
}

flattened:

{
  "user.id": 1,
  "user.name": "John Smith",
  "user.contact.email": "[email protected]"
  "user.contact.phone": "07123 456789"
}

So the unflattened JSON would always be like

{
    "id": 1,
    "name": "John Smith",
    "email": "[email protected]",
    "phone": "07123 456789" 
  }

Thus, the flattened JSON would never need a . in the key and can directly use the key.

@Bisht13 Bisht13 force-pushed the feat/add-proof-soulbound-nft branch from 715bad3 to af6592a Compare November 29, 2024 15:01
@Bisht13 Bisht13 changed the base branch from main to feat/refactor November 29, 2024 15:02
@JohnGuilding
Copy link
Author

@Bisht13 there are a few different ways we could do flattened json. The reason I am skeptical of the approach you outlined is we loose information. In this case, user and contact. This could be problematic for larger and more complex json structures (I'm also not sure how large these json structures would get, maybe they would always be small?).

If nested json structures are not expected, then your solution makes sense.

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.

2 participants