SmartTokenLabs / attestation

Paper and implementation of blockchain attestations
MIT License
48 stars 10 forks source link

Create LisconNFT minter based on TokenAttestation object #207

Closed oleggrib closed 2 years ago

oleggrib commented 2 years ago

Contract-related tasks: 0a. Our hosting server fire LisconNFT smartContract listener. 0b. We create LisconNFT smartContract image and save to the hosting ( @AW-STJ , can you ask someone to create image and contractMeta content? ) 0c. We create LisconNFT smartContract metadata JSON and save to the hosting (@foxgem , can you provide URL what is OK for you?)

How it will work:

  1. User visit LisconTickets website
  2. LisconTickets Negotiator check if user have tickets, 2a. no tickets. Message kind of: "Please check your mailbox and click magic link if you had a Liscon ticket."
  3. Display tickets slider.
  4. User can click a ticket and code will create emailAttestation based on magicLink
  5. Negotiator request Authenticator to create useToken object (ticket+emailAttestation+secretOwnershipProof)
  6. LisconTickets requests LisconNFT smartContract to validate useToken.
  7. LisconTickets sends useToken to the LisconNFT smartContract mint(useToken) function
  8. LisconNFT sends useToken to ValidateAttestation smart contract to validate attestation. It returns [subjectAddress, ticketID, valid] ( @JamesSmartCell , are you working on that validator? can you do it? )
  9. if object valid then LisconNFT mints token with id=ticketID and set subjectAddress as token owner
  10. Hosting Listener receive LisconNFT Transfer() event and create next files: 10a. generate TokenImage from template ( @AW-STJ , we need some template for token image. We can use tokenID to generate unique images, @foxgem , cant we use enpoint like you created for Derived contracts to save image/JSON ? ) 10b. generate TokenMetadata JSON ( @AW-STJ , we need tokenMetaData template )

ERC721 LisconNFT SmartContract code is ready. Website flow is ready too.

Most challenging part is create Validation Smart Contract.

@AW-STJ , what blockchain will we use for Liscon tokens? point me please to the Issue for that task in case if we already have one.

Any comments, updates, suggestions? @zhangzhongnan928 @colourful-land @JamesSmartCell ?

oleggrib commented 2 years ago

Currently in Attestation lib for Attested Object we use EqualityProof. It use curvePoints substraction and other gas-expencive operations to validate proof. Its ok for regualer server/browser, but too expencive for SmartContract

https://github.com/witnet/elliptic-curve-solidity this is example lib for curvePoints manipulations in solidity . every operation costs lot of gas. we can try to implement verify proof with solidity and estimate gas usage or try to make objects simplier for solidity.

any sugestions @JamesSmartCell @colourful-land @jot2re ?

foxgem commented 2 years ago

0c. We create LisconNFT smartContract metadata JSON and save to the hosting (@foxgem , can you provide URL what is OK for you?)

let's follow the same rules used in autographnft, here is new rule proposal which needs the decision from @colourful-land

Hosting Listener receive LisconNFT Transfer() event and create next files: 10a. generate TokenImage from template ( @AW-STJ , we need some template for token image. We can use tokenID to generate unique images, @foxgem , cant we use enpoint like you created for Derived contracts to save image/JSON ? ) 10b. generate TokenMetadata JSON ( @AW-STJ , we need tokenMetaData template )

yes, we can and this will be one of development tasks.

jot2re commented 2 years ago

Currently in Attestation lib for Attested Object we use EqualityProof. It use curvePoints substraction and other gas-expencive operations to validate proof. Its ok for regualer server/browser, but too expencive for SmartContract

https://github.com/witnet/elliptic-curve-solidity this is example lib for curvePoints manipulations in solidity . every operation costs lot of gas. we can try to implement verify proof with solidity and estimate gas usage or try to make objects simplier for solidity.

any sugestions @JamesSmartCell @colourful-land @jot2re ?

If we are fine with a single use of the emailAttestation then we could follow the same flow/idea as @oleggrib suggested for alchemyNFT, where attestation.id validates the user's email via the magicLink and if satisfied signs a useToken object that is time-limited. The user can then post this to the smart contract. Using such a flow, the validation ensuring that the user's ticket is issued to the user posting the useToken to the smart contract, is delegated to attestation.id and hence it will not be necessary to include a ZKP in what is posed to the smart contract. However, this of course changes the object we already have quite a bit.

oleggrib commented 2 years ago

@jot2re , I had a chat with @JamesSmartCell , he found a way to validat ZK proof and use 77k of gas , so we can wait for final verification object and estimate whole object validation. Then we can decide what to do.

JamesSmartCell commented 2 years ago

This is for the verifyEqualityProof, it doesn’t include parsing the full SignedIdentifierAttestation and parsing commitments1, 2 and POK out of the TicketAttestation and checking the signatures. My gut feeling is around 120K for a full validation, which is still better than Zokrates' ~300K for equivalent (they may have improved since last year when I last checked).

I am coding that in today, and should have it ready later today or Saturday (off hiking on Friday). Because we already have the parsing code for IdentifierAttestation from Alchemy is should be a modular plug, play and fiddle to get it going. Plus some modernisation of the contracts since we've gone up a level in Solidity since last year.

JamesSmartCell commented 2 years ago

@jot2re @oleggrib Should I also be checking the equalityProof (com1, com2, pok), or just the EC-Signatures? Or have two functions, one that checks the equalityProof and one that just pulls out the recovered signatures?

JamesSmartCell commented 2 years ago

Initially, I'll commit the version that extracts the three key addresses. The function to verifyEquality is there and working, but not hooked up to anything. What kind of test would show this working?

jot2re commented 2 years ago

Initially, I'll commit the version that extracts the three key addresses. The function to verifyEquality is there and working, but not hooked up to anything. What kind of test would show this working?

@JamesSmartCell the reference flow for testing a valid attested ticket request is here basically this validates the underlying AttestedObject (Ticket and Identifier Attestation) and that the rest of the EIP712 is as expected. So the meat you really want is here and here. Thus I think the way to adapt is to adapt the test to use a simulated smart contract as done in this test

jot2re commented 2 years ago

@jot2re @oleggrib Should I also be checking the equalityProof (com1, com2, pok), or just the EC-Signatures? Or have two functions, one that checks the equalityProof and one that just pulls out the recovered signatures?

If we go with the flow we have now it should both check the signatures and the ZKP. If we change the flow as suggested above (and trust attestation.id to check the ZKP) then we can avoid validating it in the smart contract.

JamesSmartCell commented 2 years ago

@jot2re @oleggrib Should I also be checking the equalityProof (com1, com2, pok), or just the EC-Signatures? Or have two functions, one that checks the equalityProof and one that just pulls out the recovered signatures?

If we go with the flow we have now it should both check the signatures and the ZKP. If we change the flow as suggested above (and trust attestation.id to check the ZKP) then we can avoid validating it in the smart contract.

Got it, I'll add two functions; one that only checks the ec-sigs, and a second that checks ZKP, and a third that checks both. That way we can pick and choose which to use depending on gas cost.

I already have the gas cost for the ZKP, about 77K, I'll have the ec cost later today.

JamesSmartCell commented 2 years ago

@oleggrib @jot2re Gas cost for full verification - that is, checking issuer & attestor signatures, extracting subject address and finally verifying the ZKP is 128k. So, not too bad, and any subset of these checks will be better.

I'll check in the contract and hook up the test.

Do we want test subsets? That is, only ec-recover and extract subject address, or only ZKP?

JamesSmartCell commented 2 years ago

The first working contract commit is in the 192_vulnerability branch

Can you let me know about the test subsets?

I'm a bit confused about these; what would be the flow if we're using the ZKP's?

I would have thought that you would only need one type of validation or the other? Wouldn't the ZKP only work if the Proof was generated using the correct key?

JamesSmartCell commented 2 years ago

The contract now returns the TicketID along with the three addresses:

function verifyTicketAttestation(bytes memory attestation) public view returns(address attestor, address ticketIssuer, address subject, bytes memory ticketId)
jot2re commented 2 years ago

@jot2re @oleggrib Should I also be checking the equalityProof (com1, com2, pok), or just the EC-Signatures? Or have two functions, one that checks the equalityProof and one that just pulls out the recovered signatures?

If we go with the flow we have now it should both check the signatures and the ZKP. If we change the flow as suggested above (and trust attestation.id to check the ZKP) then we can avoid validating it in the smart contract.

Got it, I'll add two functions; one that only checks the ec-sigs, and a second that checks ZKP, and a third that checks both. That way we can pick and choose which to use depending on gas cost.

I already have the gas cost for the ZKP, about 77K, I'll have the ec cost later today.

Sounds good, just be aware that if we go with another flow the data objects will be different. But in any case it is good to have the code framework for it already programmed ;)

jot2re commented 2 years ago

The first working contract commit is in the 192_vulnerability branch

Can you let me know about the test subsets?

I'm a bit confused about these; what would be the flow if we're using the ZKP's?

I would have thought that you would only need one type of validation or the other? Wouldn't the ZKP only work if the Proof was generated using the correct key?

@JamesSmartCell would it be possible to work on a new branch out of the master? 192 was merged over a month ago and the master branches has had several other merges.

I am unsure of what you mean about only needing to validate the ZKP? The signatures on the ticket and attestation must also be validated to ensure that they have been issued by the correct authority. The ZKP only shows that the commitment in the Ticket and IdentifierAttestation is to the same email.

I am also unsure of what you mean with subtests; in general it would be optimal if what is tested on the smart contract is the same as what is tested on the backend code, i.e, the tests here.

JamesSmartCell commented 2 years ago

The first working contract commit is in the 192_vulnerability branch Can you let me know about the test subsets? I'm a bit confused about these; what would be the flow if we're using the ZKP's? I would have thought that you would only need one type of validation or the other? Wouldn't the ZKP only work if the Proof was generated using the correct key?

@JamesSmartCell would it be possible to work on a new branch out of the master? 192 was merged over a month ago and the master branches has had several other merges.

OK

I am unsure of what you mean about only needing to validate the ZKP? The signatures on the ticket and attestation must also be validated to ensure that they have been issued by the correct authority. The ZKP only shows that the commitment in the Ticket and IdentifierAttestation is to the same email.

Ah got it, so we can ensure it's not a portmanteau of different ticket and identifierattestation. Ok that makes sense.

I am also unsure of what you mean with subtests; in general it would be optimal if what is tested on the smart contract is the same as what is tested on the backend code, i.e, the tests here.

With the explanation above I realise it has to be all done together - thanks for the clarification.

oleggrib commented 2 years ago

Hi, @JamesSmartCell , we have minor incompatibility between Liscon ticket and new Ticket format. Liscon ticket includes commitment as unsigned part:

SEQUENCE (3 elem)
   UTF8String 26
   INTEGER (79 bit) 36
   INTEGER 1
OCTET STRING (65 byte) 042CB…
BIT STRING (520 bit) 000…

but new Ticket format embeds commitment in the signed sequence

SEQUENCE (4 elem)
   UTF8String Åø
   INTEGER (119 bit) 5460484456...
   INTEGER 0
   OCTET STRING (65 byte) 04216…
BIT STRING (520 bit) 001…

can you add format version as input to allow both formats(maybe more formats in the future)?

jot2re commented 2 years ago

Currently only the version WITH commitment signed (i.e. the "good" and new one) can be used in the smart contract.

oleggrib commented 2 years ago

We can close this issue, nothing to do here