search

metaplex-foundation / sugar

Candy Machine Rust CLI.
Apache License 2.0
203 stars 116 forks source link

[Bug]: sugar airdrop is not honoring the airdrop json. #432

Closed VFe closed 1 year ago

VFe commented 1 year ago

Issue description

  1. Get an airdrop json together(say, 10,000 total items in my case).
  2. Run, several dozen transactions failed. Program states to re-run.
  3. Those failed transactions result in a double sending of some. Ex. A person was supposed to get 34, but got 36.

Can share relevant logs, outputs, and transactions if directed.

I'm not entirely sure #3 is the cause or a red herring, speculating slightly

Relevant log output

No response

Priority this issue should have

High - Core Feature not being reliable.

samuelvanderwaal commented 1 year ago

When you ran it, did it create an airdrop_results.json file?

VFe commented 1 year ago

When you ran it, did it create an airdrop_results.json file?

Yes it did.

VFe commented 1 year ago

Let me know what details are the most helpful to share and I can get them together

samuelvanderwaal commented 1 year ago

I haven't been able to duplicate this so far on simple tests. If I re-run an airdrop with an existing airdrop_results.json file that has transactions stored I get:

Selection_144

Did you delete the airdrop_results.json file or run the command from a different directory?

samuelvanderwaal commented 1 year ago

I suppose what may have happened is the RPC was unsure if the transaction went through and gave an error so it was recorded in the airdrop file as a failure even though it actually went through. That happens sometimes when you get an error like "Transaction was not confirmed in 30.00 seconds. It is unknown if it succeeded or failed."

We made need to check for that specific error and handle it appropriate, but that seems a bit complicated.

VFe commented 1 year ago

To add additional detail. I believe what you said above is correct ^ I'm able to re-create this consistently.

Run it. After it finishes and states some failures, re-run it again. In my case there was a lot of those "not confirmed" types, and those resulted in double sendings because presumably they were accounted for in the results.json as failures but succeeded.

As a user who had my project/mint get a lot of drama over this after selling out , I would say my preferred behavior would always be error on the side of caution.

i.e. an unclear failure/success state should be counted as a success in the bookkeeping. It'd be somewhat complicated to actually "follow up" the transactions to verify, but from the perspective of a user, 100/100 times I think most would prefer the situation where the tool says "hey I didn't send these because I'm unsure, please manually audit these 500 transactions" then something that results in a double sending.

samuelvanderwaal commented 1 year ago

@VFe I've opened a PR to address this, if you get a chance to test the PR that would be appreciated.

https://github.com/metaplex-foundation/sugar/pull/439

samuelvanderwaal commented 1 year ago

Let me know if the fix doesn't solve the issue for you.

santicevic commented 1 year ago

@samuelvanderwaal this happened to us yesterday, instead of minting out 500 nfts we ended up minting 654 airdrop_results.json was generated almost all of the nfts that were marked as failed were minted in the first go

samuelvanderwaal commented 1 year ago

@samuelvanderwaal this happened to us yesterday, instead of minting out 500 nfts we ended up minting 654 airdrop_results.json was generated almost all of the nfts that were marked as failed were minted in the first go

Are you on the latest Sugar: v2.1.1?