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

New protocol design #2: Refactor of safe and whitelisting #86

Merged
merged 10 commits into from
Nov 24, 2022

Conversation

porkbrain
Copy link
Contributor

No description provided.

@porkbrain porkbrain self-assigned this Nov 21, 2022
/// Witness is a type always in form "struct Witness has drop {}"
///
/// They must be from the same module for this assertion to be ok.
public fun assert_same_package_as_witness<OneTimeWitness, Witness>() {
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be assert_same_module_as_witness, as we also check module.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

renamed


// NFT object with an option to hold `D`ata object
struct Nft<phantom T, D: store> has key, store {
struct NFT<phantom C> has key, store {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I like NFT better than Nft, but I think if we name the struct the same name of the module but in Caps, it will make it a OTW. Perhaps we keep it Nft?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I believe an OTW must have drop only and no fields.

Comment on lines -205 to -222
/// Remove an NFT from the `Safe` and give it back to the logical owner.
public entry fun withdraw_nft<T, D: store>(
nft_id: ID,
owner_cap: &OwnerCap,
safe: &mut Safe,
) {
assert_owner_cap(owner_cap, safe);
assert_contains_nft(&nft_id, safe);

let (_, ref) = vec_map::remove(&mut safe.refs, &nft_id);
// an exlusively listed NFT must first have its transfer cap burned
// via fun burn_transfer_cap
assert_not_exlusively_listed(&ref);

let nft = object_bag::remove<ID, Nft<T, D>>(&mut safe.nfts, nft_id);
nft::transfer_to_owner(nft);
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

Is the intention to add back this function later?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We don't want to allow the user to freely take an NFT out of a safe at the current design stage, which is what this function was for.

sources/safe/royalties.move Outdated Show resolved Hide resolved
Comment on lines 74 to 84
/// Only the designated witness can access the balance.
///
/// Typically, this would be a witness exported from the collection contract
/// and it would access the balance to calculate the royalty in its custom
/// implementation.
public fun balance_mut<C, W: drop, FT>(
_witness: W,
payment: &mut TradePayment<C, W, FT>,
): &mut Balance<FT> {
&mut payment.amount
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is it needed &mut balance for calculating the royalty in its custom implementation, or should &balance suffice?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Isn't this only needed when transferring the funds to the beneficiary?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You need access to &mut balance to be able to actually charge the royalty. Ie. you calculate it from this balance and then you take your cut out of it.

@porkbrain porkbrain merged commit 3877a14 into feature/safe Nov 24, 2022
@porkbrain porkbrain deleted the refactor/new-nft-design branch December 20, 2022 06:58
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.

3 participants