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

Add user ability to unlock with authorized deposit #82

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

loic1
Copy link
Contributor

@loic1 loic1 commented Oct 16, 2024

Changes:

  • Update NFTLocker contract to support the ability to force unlock an NFT with authorized deposit in a generic manner
    • allow the admin to add/remove authorized deposit handlers
    • allow the user to unlock with authorized deposit on their own
  • Update Escrow contract to define an interface-conforming deposit handler for NFTLocker
  • Add admin and user transactions and tests

The changes are purely additive, they are not intended to affect any existing on-chain setup.

/// only the admin account can add or remove receivers - in the future, a ReceiverProvider resource could
/// be added to provide this capability to separate authorized accounts.
///
access(all) resource ReceiverCollector {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

New admin resource to store authorized deposit handlers - this allows separating concerns with the existing Admin resource, and it's also not possible to add new fields into the existing Admin resource (not a valid change for contract update).

The resource is meant to be stored at a specific path in the admin account storage to allow certain methods to be accessible publicly via borrowAdminReceiverCollectorPublic.

///
/// Receivers are entities that can receive locked NFTs and deposit them using a specific deposit method
///
access(all) struct Receiver {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

New struct meant to organize how deposit handlers are stored in the admin's ReceiverCollector resource


/// Interface for depositing NFTs to authorized receivers
///
access(all) struct interface IAuthorizedDepositHandler {
Copy link
Contributor Author

@loic1 loic1 Oct 16, 2024

Choose a reason for hiding this comment

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

New struct interface to ensure a valid deposit method to be used for the receiver in unlockWithAuthorizedDeposit is defined

Note: It is not possible to store functions (original approach).


/// Expire lock if the locked NFT is eligible for force unlock and deposit by authorized receiver
///
access(contract) fun expireLock(id: UInt64, nftType: Type) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

expireLock definition is moved at the contract level (with access(contract)) in order to be called from unlockWithAuthorizedDeposit, if authorized

/// additional function parameters may be required by the receiver's deposit method and are passed in the
/// passThruParams map.
///
access(Operate) fun unlockWithAuthorizedDeposit(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

unlock a locked NFT if an authorized deposit method has been defined (e.g., deposit into escrow leaderboard) - no need for admin intervention

@@ -231,6 +232,22 @@ access(all) contract Escrow {
}
}

// Handler for depositing NFTs to the Escrow Collection, used by the NFTLocker contract.
access(all) struct DepositHandler: NFTLocker.IAuthorizedDepositHandler {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Deposit handler definition to unlock a locked NFT and add into escrow leaderboard


/// This transaction creates a new ReceiverCollector resource and adds a new receiver to it with a deposit method that adds an NFT to an escrow leaderboard.
///
transaction() {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Admin tx to set up escrow leaderboard receiver


/// This transaction unlocks the NFT with provided ID and adds to an escrow leaderboard, unlocking them if necessary.
///
transaction(leaderboardName: String, nftID: UInt64) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

User transaction to unlock a locked NFT and directly add into escrow leaderboard

@loic1 loic1 marked this pull request as ready for review October 16, 2024 00:49
@loic1 loic1 requested a review from a team as a code owner October 16, 2024 00:49
@@ -123,52 +277,93 @@ access(all) contract NFTLocker {
access(all) view fun getIDs(nftType: Type): [UInt64]?
access(Operate) fun lock(token: @{NonFungibleToken.NFT}, duration: UInt64)
access(Operate) fun unlock(id: UInt64, nftType: Type): @{NonFungibleToken.NFT}
access(Operate) fun unlockWithAuthorizedDeposit(
Copy link
Collaborator

Choose a reason for hiding this comment

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

nice!

?? panic("Could not borrow a reference to the public leaderboard collection")

// Add the NFT to the escrow leaderboard
escrowCollectionPublic.addEntryToLeaderboard(nft: <-nft, leaderboardName: leaderboardName, ownerAddress: ownerAddress)
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

Deewai
Deewai previously approved these changes Oct 16, 2024
Copy link
Collaborator

@Deewai Deewai left a comment

Choose a reason for hiding this comment

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

Great work!

@loic1 loic1 requested a review from Deewai October 16, 2024 17:07
Deewai
Deewai previously approved these changes Oct 17, 2024
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