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

Feature/fix sdk price error #326

Open
wants to merge 6 commits into
base: develop
Choose a base branch
from
Open

Conversation

Yashiru
Copy link
Contributor

@Yashiru Yashiru commented Jun 5, 2024

Context

This PR fixed the price error issue found in the SDK, and adds a test to verify the fix for the transfer issue in HolographDropERC721V2.
This PR also added some tests to highlight the fact thats the issue occurs also on all other chains, but the block explorer doesn't display failed transfer that are handled by the code.

Transactions that has failed transfers

Sepolia
OP Sepolia
Base Sepolia
Zora Sepolia
Arbitrum Sepolia
BNB
Fuji

Describe Changes

  • Fixed the price error that was identified in the SDK
  • Added a test to be able to confirm that the fix has solved the issue, based on an existing sepolia transaction, replacing the bytecode of the bugger contract.
  • Added some tests to confirm that the bug occurs on each chain by reproducing the above transactions to highlight the OutOfFunds error using foundry

Tests overview

The goal of the tests is to reproduce the exact same transaction as the one failing on sepolia and to reproduce it a second time with the new bytecode of the HolographDropERC721V2 to see if it fixes the issue.

test_V2PurchaseFreeMoeWithOldVersion

  • Reproduces a transaction from block 6031723 using the old contract version.
  • The transfer fails with EvmError: OutOfFunds

test_V2PurchaseFreeMoeWithNewVersion

  • Reproduces the same transaction with the new HolographDropERC721V2 fixed bytecode.
  • The transfer no longer fails with EvmError: OutOfFunds

All other tests

  • Reproduces transaction for each chain at the right block, with the right value
  • The transfer fails with EvmError: OutOfFunds

Steps to run tests

Note

You need to use a forks networks at the same block as each transaction to reproduce to replicate the exact same behavior, you can use tenderly to create forks at custom blocks.
e.g. https://rpc.tenderly.co/fork/[YOUR_FORK_UUID]

👉 Run the first test

This test reproduce the exact bugged sepolia transaction

forge test --mt test_V2PurchaseFreeMoeWithOldVersion -vvvv

You shoud see that the last transfer to 0xa57106357F9A487F6AfBaA3758e7fCcB787113c4 is failing
Capture d’écran 2024-06-05 à 15 24 35

👉 Run the second test

This test reproduce the exact [bugged sepolia transaction] but with the old bytecode replaced by new fixed one.(https://sepolia.etherscan.io/tx/0xb8dcf96afafb93d9148c1c469712ec37be17b8913a68f2c1a1d371a18d1194fb)

forge test --mt test_V2PurchaseFreeMoeWithNewVersion -vvvv

You shoud see that the last transfer to 0xa57106357F9A487F6AfBaA3758e7fCcB787113c4 is not failing anymore
Capture d’écran 2024-06-05 à 15 31 08




Run all other tests

Running all other test allow us to confirm that the issue also occurs on other network.

forge test --mt [TEST_NAME_TO_RUN] -vvvv

You shoud see that the last transfer to 0xa57106357F9A487F6AfBaA3758e7fCcB787113c4 is failing
Capture d’écran 2024-06-05 à 15 24 35

Checklist before requesting a review

  • I have performed a self-review of my code
  • Code styles have been enforced
  • All Foundry tests are passing

@Yashiru Yashiru self-assigned this Jun 5, 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.

1 participant