Skip to content
This repository has been archived by the owner on Oct 25, 2024. It is now read-only.

Investigate failing TransferOut receipt test #1276

Closed
deekerno opened this issue Aug 16, 2023 · 3 comments · Fixed by #1356
Closed

Investigate failing TransferOut receipt test #1276

deekerno opened this issue Aug 16, 2023 · 3 comments · Fixed by #1356
Assignees
Labels
bug Something isn't working good first issue Good for newcomers

Comments

@deekerno
Copy link
Contributor

The TransferOut receipt test currently fails as the contract is unable to transfer the asset to an address. This should be fixed to ensure that our E2E test suite properly covers all receipts.

@deekerno deekerno added bug Something isn't working good first issue Good for newcomers labels Aug 16, 2023
@dadepo
Copy link
Contributor

dadepo commented Sep 3, 2023

@deekerno I am thinking of looking into this next. If you've got any more background or pointers/ideas as to why the test is flaky, please do share.

@deekerno
Copy link
Contributor Author

deekerno commented Sep 3, 2023

Hey @dadepo! Thanks for your willingness to look into this!

I don't think there's much background that isn't used as part of the test and the fixture themselves, but I'll share some information:

  • This test uses the transfer_to_address Sway method in order to send an asset to an address. The indexer's end-to-end tests use the Rust SDK to trigger the custom Sway functions and create the necessary receipts. Since it generates an output, the fixture has historically used the .append_variable_outputs(...) method in the SDK call and since there is a recipient to transferred assets, the Sway method itself needs to be annotated with #[payable] in the contract.
    -- Both of these requirements have already been fulfilled.
  • I don't have any logs or an ability to check the exact error at this time, but as far as I remember, the tests fails because of an OutOfGas error. We typically set the amount of available gas to an inordinate amount to prevent these errors, but it seems that there may have been some sort of change in how this test should be constructed.
  • My pointers:
    -- Ensure the Sway transfer method is correctly formed: It's possible that the recommended way to transfer to an address has changed.
    -- Check if the process to call contract methods from the SDK has changed: For this, I'd look at the documentation for the SDK or check the changelogs for fuels-rs to see if anything concerning contract methods has been changed.
    -- Mind the contract_id field in the test manifest: If you change the Sway contract in any way, its contract ID may change. If so, you'll need to determine the new contract ID and change the test manifest file so the test indexer correctly indexes the contract.

A successful implementation should have a correctly functioning TransferOut test and will have the #[ignore] directive removed so that it is included in our test suite again.

If you need any help, let us know. Thanks! 😄

@dadepo
Copy link
Contributor

dadepo commented Sep 17, 2023

@deekerno Thanks for the pointers! it was really helpful :)

Just opened a PR which I believe fixes the issue here #1356

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants