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

fix: RPC starkNet_executeTxn storing in-correct state data if the params calls is not an array #376

Merged
merged 1 commit into from
Oct 11, 2024

Conversation

khanti42
Copy link
Collaborator

@khanti42 khanti42 commented Oct 10, 2024

PR Description:

This PR addresses the following changes:

  1. Fixture Update:

    • The callsExamples.json fixture has been updated to handle both cases where calls can be either an array or a single object. This ensures that the test scenarios are consistent with the real-world behavior where calls might be in different formats.

    Changes:

    • Previously, calls were always treated as an array. The second example in the fixture now provides calls as a single object, allowing the tests to cover this scenario.
  2. Test Enhancements:

    • Refactored the executeTxn.test.ts tests to use it.each to handle both cases where calls could be either an array or a single object.
    • This approach simplifies the test by parameterizing both cases (Call[] and Call), ensuring that the code is properly tested for different scenarios without duplicating test logic.

    Changes:

    • The previous test for executeTxn has been replaced with an it.each block, covering both the array and object formats of calls.
    • The test correctly asserts that the transaction execution behaves as expected, regardless of the format of calls.
  3. Code Improvement:

    • The executeTxn.ts logic was updated to handle both array and non-array formats for calls.
    • A new variable call was introduced to check if calls is an array and, if so, uses the first element; otherwise, it directly uses the object. This avoids potential issues when calls is not an array.

    Changes:

    • The line calls[0] in the addTransaction method was replaced with the newly created call variable, ensuring correct handling of both formats.

Summary:

This PR ensures that both the code and tests properly handle calls when it is an array or a single object, improving robustness and coverage in different scenarios.

@khanti42 khanti42 requested a review from a team as a code owner October 10, 2024 11:44
@khanti42 khanti42 requested review from Akaryatrh and stanleyyconsensys and removed request for a team October 10, 2024 11:44
packages/starknet-snap/src/rpcs/executeTxn.ts Outdated Show resolved Hide resolved
packages/starknet-snap/src/rpcs/executeTxn.test.ts Outdated Show resolved Hide resolved
packages/starknet-snap/src/rpcs/executeTxn.test.ts Outdated Show resolved Hide resolved
packages/starknet-snap/src/rpcs/executeTxn.test.ts Outdated Show resolved Hide resolved
packages/starknet-snap/src/rpcs/executeTxn.test.ts Outdated Show resolved Hide resolved
@stanleyyconsensys stanleyyconsensys changed the title fix: execute should accept single call fix: RPC starkNet_executeTxn storing in-correct tx data when single calls argument is given Oct 10, 2024
@stanleyyconsensys stanleyyconsensys changed the title fix: RPC starkNet_executeTxn storing in-correct tx data when single calls argument is given fix: RPC starkNet_executeTxn storing in-correct state data when single calls argument is given Oct 10, 2024
@stanleyyconsensys stanleyyconsensys changed the title fix: RPC starkNet_executeTxn storing in-correct state data when single calls argument is given fix: RPC starkNet_executeTxn storing in-correct state data if single calls argument was given Oct 10, 2024
@stanleyyconsensys stanleyyconsensys changed the title fix: RPC starkNet_executeTxn storing in-correct state data if single calls argument was given fix: RPC starkNet_executeTxn storing in-correct state data if calls argument is not an array Oct 10, 2024
@stanleyyconsensys stanleyyconsensys changed the title fix: RPC starkNet_executeTxn storing in-correct state data if calls argument is not an array fix: RPC starkNet_executeTxn storing in-correct state data if the params calls is not an array Oct 10, 2024
@khanti42 khanti42 force-pushed the fix/execute_should_accept_single_call branch 2 times, most recently from 78f8573 to 840ea4e Compare October 10, 2024 12:47
@khanti42 khanti42 force-pushed the fix/execute_should_accept_single_call branch from 840ea4e to dcff7de Compare October 10, 2024 12:54
packages/starknet-snap/src/rpcs/executeTxn.test.ts Outdated Show resolved Hide resolved
packages/starknet-snap/src/rpcs/executeTxn.test.ts Outdated Show resolved Hide resolved
packages/starknet-snap/src/rpcs/executeTxn.test.ts Outdated Show resolved Hide resolved
packages/starknet-snap/src/rpcs/executeTxn.test.ts Outdated Show resolved Hide resolved
@khanti42 khanti42 force-pushed the fix/execute_should_accept_single_call branch from dcff7de to 8051fb1 Compare October 10, 2024 13:26
@khanti42 khanti42 force-pushed the fix/execute_should_accept_single_call branch from 8051fb1 to 61a35fc Compare October 10, 2024 14:14
Copy link

sonarcloud bot commented Oct 10, 2024

Quality Gate Passed Quality Gate passed for 'consensys_starknet-snap-wallet-ui'

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

Copy link

sonarcloud bot commented Oct 10, 2024

Quality Gate Passed Quality Gate passed for 'consensys_starknet-snap-starknet-snap'

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
37.9% Duplication on New Code

See analysis details on SonarCloud

Copy link
Collaborator

@stanleyyconsensys stanleyyconsensys left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thank you very much for the update

@khanti42 khanti42 added this pull request to the merge queue Oct 11, 2024
Merged via the queue into main with commit 508b958 Oct 11, 2024
13 checks passed
@khanti42 khanti42 deleted the fix/execute_should_accept_single_call branch October 11, 2024 08:00
@github-actions github-actions bot mentioned this pull request Oct 10, 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.

2 participants