starknet-devnet-rs icon indicating copy to clipboard operation
starknet-devnet-rs copied to clipboard

Improve testing of starknet_simulateTransactions

Open FabijanC opened this issue 2 years ago • 3 comments

Tests in crates/starknet-devnet/tests/test_simulate_transactions.rs were written to have the same coverage as devnet-py the corresponding devnet-py tests. Currently there are TODOs:

  • Better testing of SKIP_FEE_CHARGE by using a greater than zero max_fee in several tests
  • Add testing of invoke tx simulation being reverted (could be done by using an invalid max_fee (checked: this leads to a Reverted case that is currently not tested)) ~~- Another test needs replacement of .ge with .gt (basically we would want to assert that using SKIP_VALIDATE actually lowers the estimated fee, but currently it does not have effect, as described in this issue: https://github.com/lambdaclass/starknet_in_rust/issues/1051)~~
    • ~~This issue can be addressed separately as it would not merely be enhancement but fix~~
  • Use starknet.rs's .simulate(...) instead of manually constructing simulation requests.

FabijanC avatar Oct 05 '23 09:10 FabijanC

Hey there! This sounds like a good first issue to work on and try my rust skills! Could I get assigned to it? I also understand Python so checking out the devnet-py test will be easy.

joseSalazar4 avatar Apr 22 '24 12:04 joseSalazar4

@joseSalazar4 you've been assigned

FabijanC avatar Apr 22 '24 13:04 FabijanC

Try adding two more tests:

  • one which simulates a tx that gets reverted (and asserting that this is what really happened) - find a way to make it revert, there might be some helpful clues in the TODOs
  • one which tests SKIP_FEE_CHARGE in conditions that haven't yet been tested in the file

If you feel the tests are too similar, then just add one.

In any case, feel free to create at least a draft PR even if you're not yet done (but indicating so). That way we can see exactly what you've done so far and make concrete suggestions.

FabijanC avatar Apr 30 '24 07:04 FabijanC