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

[CNft] CNft Collection Offer #107

Merged
merged 36 commits into from
Nov 22, 2024

Conversation

JeremyLi28
Copy link
Contributor

@JeremyLi28 JeremyLi28 commented Oct 24, 2024

Implemented sol_cnft_fulfill_buy

It's

  • Support allowlist by mcc
  • Royalty enforced
  • Support all other functionalities for collection offer: shared escrow, reinvest etc

@JeremyLi28 JeremyLi28 changed the title Chen/fulfill cnft buy [CNft] CNft Collection Offer Nov 9, 2024
programs/mmm/src/util.rs Outdated Show resolved Hide resolved
}

#[derive(AnchorSerialize, AnchorDeserialize, Clone)]
pub struct MetadataArgs {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

look into if we can pass in serialized data

Copy link
Contributor Author

Choose a reason for hiding this comment

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

tried with serialize and deserialize, the deserilized data has some unknown issue that I couldn't figure out in short time (it fails allowlist check, means the deserialization had some issue). will keep the metadata args as it is for now to unblock this PR

programs/mmm/src/instructions/cnft/sol_cnft_fulfill_buy.rs Outdated Show resolved Hide resolved
@@ -1124,6 +1231,152 @@ pub fn create_core_metadata_core(royalties: &Royalties) -> MplCoreMetadata {
}
}

#[allow(clippy::too_many_arguments)]
pub fn transfer_compressed_nft<'info>(
Copy link
Contributor

Choose a reason for hiding this comment

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

nitpick: you can use the mpl-bubblegum TransferCpiBuilder for the CPI call here
https://github.com/metaplex-foundation/mpl-bubblegum/blob/4d35cec5ac6e72690e164725b6106d4ef610ebe0/clients/rust/src/generated/instructions/transfer.rs#L433
used something similar in the CMX mint_nft_core ix if you want to look at that for reference

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah I'm aware of that, just try to save time reuse the code from m3, added a todo though

return Err(MMMErrorCode::InvalidRequestedPrice.into());
}

anchor_lang::solana_program::program::invoke_signed(
Copy link
Contributor

Choose a reason for hiding this comment

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

anchor 0.29 added support for transferring from PDAs
you can do the following

let buyside_sol_escrow_account = &ctx.accounts.buyside_sol_escrow_account;
buyside_sol_escrow_account.sub_lamports(payment_amount)?;
payer.add_lamports(payment_amount)?;

see https://github.com/coral-xyz/anchor/pull/2552/files#diff-c30312f654589dc8120c3ca5d75db6816b7825627c10c7c4b485215991bd54e7

Copy link
Contributor Author

Choose a reason for hiding this comment

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

seems like we can't change lamports for payer in this case since we don't own it?

Message: Transaction simulation failed: Error processing Instruction 0: instruction spent from the balance of an account it does not own. 
    Logs: 
    [
      "Program cmtDvXumGCrqC1Age74AVPhSRVXJMd8PJS91L8KbNCK consumed 41544 of 113844 compute units",
      "Program cmtDvXumGCrqC1Age74AVPhSRVXJMd8PJS91L8KbNCK success",
      "Program BGUMAp9Gq7iTEuizy4pqaxsTyUCBK68MDfK752saRPUY consumed 58978 of 130619 compute units",
      "Program BGUMAp9Gq7iTEuizy4pqaxsTyUCBK68MDfK752saRPUY success",
      "Program 11111111111111111111111111111111 invoke [2]",
      "Program 11111111111111111111111111111111 success",
      "Program 11111111111111111111111111111111 invoke [2]",
      "Program 11111111111111111111111111111111 success",
      "Program mmm3XBJg5gk8XJxEKBvdgptZz6SgK4tXvn36sodowMc consumed 139097 of 200000 compute units",
      "Program mmm3XBJg5gk8XJxEKBvdgptZz6SgK4tXvn36sodowMc failed: instruction spent from the balance of an account it does not own"
    ]. 
    Catch the `SendTransactionError` and call `getLogs()` on it for full details.

Copy link
Contributor

Choose a reason for hiding this comment

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

you can't subtract lamports from the payer since it's not owned by the program. you can subtract it from the pda

system_program,
buyside_sol_escrow_account_seeds,
)?;
try_close_sell_state(sell_state, payer.to_account_info())?;
Copy link
Contributor

Choose a reason for hiding this comment

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

not related to your PR but just FYI
if the accounts are actually typed, (e.g. the Pool and SellState), you can use the anchor close() method from the AccountsClose trait

fn try_close_sell_state(sell_state: Account<'info, SellState>, payer: AccountInfo<'info>) -> Result<()> {
  sell_state.close(payer)?
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

that's very nice, we should do a full refactor with all new optimizations

programs/mmm/src/errors.rs Outdated Show resolved Hide resolved
tests/utils/cnft.ts Outdated Show resolved Hide resolved
tests/utils/cnft.ts Outdated Show resolved Hide resolved
tests/mmm-cnft.spec.ts Outdated Show resolved Hide resolved
@nothing0012 nothing0012 merged commit 1d4ccd5 into me-foundation:main Nov 22, 2024
1 check passed
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.

4 participants