search

metaplex-foundation / metaplex-program-library

Smart contracts maintained by the Metaplex team
Other
588 stars 513 forks source link

[Bug]: Permissionless execute_sale with any wallet not working #480

Closed paul-hl closed 1 year ago

paul-hl commented 2 years ago

Which package is this bug report for?

auction-house

Which Type of Package is this bug report for?

Rust Contract

Issue description

This is a follow-up from Discord where I was told it seems to be a bug.

The auction house documentation states that the execute sale command is permissionless (if the 'requires sign-off' is false) and that anyone can call this command.

I am trying to call this command with a wallet that is neither auction house authority, buyer nor seller. And I am getting an error NoPayerPresent (6012). I tried to add the wallet as a fee payer to the transaction, I can see on solscan that I added the wallet as a fee payer to the instruction's account list, but it does not help.

I looked at the code of the Solana program and it seems that the fee payer may be either the authority or buyer or seller (https://github.com/metaplex-foundation/metaplex-program-library/blob/facd7cb11903bab086165e6619bbf0cc6e28c272/auction-house/program/src/utils.rs#L93)

I am using the latest auction house CLI.

Use case: a backend server runs the execute sale command but we do not want to have a private key of the authority stored on a server due to security reasons

Relevant log output

No response

Priority this issue should have

High (immediate attention needed)

danenbm commented 2 years ago

We will work on this next week starting Monday as it requires some coordination with another item, and will require a minor version bump because it will require adding a new account (payer) to the handler

danenbm commented 2 years ago

After discussing with team, we have two options:

  1. Either add an optional fee_payer account at the end of the execute_sale handler.
    • Pros: Invisible to users who don't need to update.
    • Cons: Involves potentially complex logic in the contract to deal with variable number of accounts in the handler. This may be worse because AH is anchor based.
  2. Add an execute_sale_V2 or execute_sale_permissionless handler that can deal with the new fee_payer account.
    • Pros: Still invisible to users who don't need to update, does not require complex logic in the contract.
    • Cons: Adds another endpoint to document and keep track of. For example, we will need to update the CLI to support this one separately. Users will need to know it exists to get the benefit.

I think the team has decided way 2 is the most straightforward at this time. We do need to coordinate these changes with current ongoing changes to AH that are in work at the moment.

We are discussing this issue at next sprint planning this week (in couple days).

NOTE: For an unrelated reason, we may need to make a similar change to sell in auction house and we can track that work with this same issue.

danenbm commented 2 years ago

Instructions update for the time being to prevent others from having issues: https://github.com/metaplex-foundation/docs/pull/266

neftworld-tn commented 2 years ago

hey guys - is permissionless execute sale on the roadmap? I'm automatically matching bids / listings serverside and would like to execute these pairs if possible. However, i cannot do so since i'm not a party to the transaction. I prefer to execute serverside and create bids / listings UI-side in order to keep transaction size down for the user.

The way I see it, this behaviour would make sense for auction houses that do not require the auction house to sign on transactions - if bids / listings can be created without the signature of the AH, then it would make sense that the same goes for execution?