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

Response errors; unexpected block execution error OutputAlreadyExists When Running Multiple Tests #89

Closed
ControlCplusControlV opened this issue Feb 8, 2022 · 10 comments · Fixed by #92
Assignees
Labels
bug Something isn't working

Comments

@ControlCplusControlV
Copy link

So was finally getting around to debugging my Sushi Sway Repo my withdraw test was failing with the following error

thread 'withdraw' panicked at 'called `Result::unwrap()` on an `Err` value: Custom { kind: Other, error: "Response errors; unexpected block execution error OutputAlreadyExists" }', /hom/me/.cargo/registry/src/github.com-1ecc6299db9ec823/fuels-rs-0.2.1/src/script.rs:28:56
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
test withdraw ... FAILED

failures:

failures:
    withdraw

It seems there seems to be some concurrency failure when running multiple tests at once. If this an error in my code though let me know and I can close the issue

@adlerjohn
Copy link
Contributor

Does this happen when running tests sequentially? My first guess is that if you run tests more than once, it'll try to deploy the same contracts again. Since they already exist in the state, you'll get that error. If can be fixed by clearning ~/.fuel and restarting your node. Or, running the node with an in-memory db: --fuel-core --db-type in-memory.

@ControlCplusControlV
Copy link
Author

It still seems to be happening even if I comment out all the other tests, rm ~/.fuel and restart my computer, not sure what is causing it then. Can you replicate?

@adlerjohn
Copy link
Contributor

Guessing this is probably an SDK issue then. cc @digorithm to triage.

@digorithm
Copy link
Member

That's odd. I always get this error when deploying a contract already deployed, as expected. And then removing the fuel dir does it for me.

Weird that removing the dir isn't resolving the issue in your case, I'll do some triaging today!

@ghost ghost assigned digorithm Feb 9, 2022
@digorithm
Copy link
Member

Update: managed to replicate, have a few ideas of what might be causing it. Will post an update soon!

@digorithm
Copy link
Member

Alright, this was a tricky one to debug, and it highlights some shortcomings in the fuel-core and the SDK. Many thanks to @Voxelot for helping me

The issue begins at this fundamental truth: every call we make at the SDK/client level generates a transaction.

That means we don't have a concept of read-only .call()-like method. Like this for instance.

That said... Sushi Sway is doing a couple of things:

  1. deposit (write)
  2. check balance ("read")
  3. withdraw (write)
  4. check the balance again ("read")

The issue here is that (2) and (4), the two checks, look like this:

let balance_of_check0 = mycontract_mod::BalanceOfInput {
    address: [10; 32],
    asset_id: [0; 32],
};

let initial_bal = contract_instance
    .balance_of(balance_of_check0) // <--- first call to balance_of
    .call()
    .await
    .unwrap();

and

let balance_of_check1 = mycontract_mod::BalanceOfInput {
    address: [10; 32],
    asset_id: [0; 32],
};

let final_bal = contract_instance
    .balance_of(balance_of_check1) // <--- second call to balance_of
    .call()
    .await
    .unwrap();

Notice that, because balance_of_check1 and balance_of_check0 are the same (same address and asset_id), the transactions generated by balance_of will be the same. Meaning the same transaction id, meaning OutputAlreadyExists.

The shortcoming on our end is that a method that's supposed to be read-only is actually generating a transaction when it shouldn't. But we don't have an API for read-only yet.

Short-term very weird workaround: make BalanceOfInput have an unused RNG-based field so that balance_of_check0 and balance_of_check1 are different and will then generate different transactions, avoiding the collision.

The actual solution, as suggested in a conversation with @Voxelot, to split the generated methods (by the abigen) between .send() (or something) that will actually create a transaction and a .call(), that underneath will use fuel-core's dry-run to not create a transaction, meaning these read-only functions can be called many times and not cause tx id collision.

cc @adlerjohn once we agree on the solution, the semantics, and the naming of .call() and .send() I'll create an issue so that we can tackle it asap.

@adlerjohn
Copy link
Contributor

Yeah, makes sense to separate call and send in the same way as Ethereum, and using dry run. Good work debugging!

@adlerjohn
Copy link
Contributor

Should this issue be transferred to fuels-rs @digorithm ?

@adlerjohn
Copy link
Contributor

Note that we also need a solution for non-view-only calls that happen to modify state rather than being inherently view-only. This could be accomplished by inserting a random coin input. Or better, a change output to a random address!

@digorithm digorithm transferred this issue from FuelLabs/sway Feb 9, 2022
@adlerjohn adlerjohn added the bug Something isn't working label Feb 9, 2022
@digorithm
Copy link
Member

Tracking this here #90

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants