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

NFT dtp agreements #221

Merged
merged 36 commits into from
Feb 8, 2022
Merged

NFT dtp agreements #221

merged 36 commits into from
Feb 8, 2022

Conversation

mrsmkl
Copy link
Contributor

@mrsmkl mrsmkl commented Feb 2, 2022

Description

Added three new agreement templates

  • NFTAccessProof: give access if somebody has the NFT.
  • NFTSlaseWithAccess: give access to data associated with NFT when buying an NFT
  • NFTAccessSwap: buy access with an NFT

TODO:

  • add all erc 721 versions
  • add unit tests

Is this PR related with an open issue?

Related to Issue #218

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • Follows the code style of this project.
  • Tests Cover Changes
  • Documentation

Funny gif

Put a link of a funny gif inside the parenthesis-->

@mrsmkl mrsmkl requested review from a team as code owners February 2, 2022 10:47
bytes32 _did,
address _lockAddress,
uint256 _amount,
address _receiver,
Copy link
Member

Choose a reason for hiding this comment

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

NFT Marked Condtion and interface looks the same than existing NFT condition and interface but without the receiver parameter. So to avoid code duplication, if receiver is necessary in the new flows, can we refactor the existing templates to include the receiver parameter? And if it's not used in some templates/flows we pass address(0)?

* can release reward if lock and release conditions
* are fulfilled.
*/
contract MultiEscrowPaymentCondition is Reward, Common, ReentrancyGuardUpgradeable {
Copy link
Member

Choose a reason for hiding this comment

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

Wouldn't be possible here to implement using the existing EscrowPaymentCondition with a new method to fulfill an array of releaseConditons? Also we could keep compatibility exposing a hash/fulfill where one releaseCondition becomes an array of releaseConditions that we pass to the implementation using the array.

@aaitor aaitor merged commit 5908789 into master Feb 8, 2022
@aaitor aaitor deleted the feat/nft-dtp-agreements branch February 8, 2022 13:56
@aaitor aaitor linked an issue Feb 10, 2022 that may be closed by this pull request
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.

Integrate DTP with NFT Access flows
2 participants