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

feat: support txv3 in estimateFee and estimateFeeBulk [SNAP] #262

Merged
merged 12 commits into from
Aug 15, 2024

Conversation

khanti42
Copy link
Collaborator

@khanti42 khanti42 commented Jun 27, 2024

This PR introduces changes related to TRANSACTION_VERSION in preparation for upcoming updates regarding txV3. The primary focus is to set a default value for transactionVersion and ensure consistency across various estimateFee and estimateFeeBulk. No changes in current functionality are introduced in this PR.

Summary of Changes:

  • Import Changes:

    • Added import of TRANSACTION_VERSION from constants in several files.
  • Code Changes:

    • estimateFee.ts:
      • Added TRANSACTION_VERSION as the default value for transactionVersion if it is not provided in requestParamsObj.
    • estimateFees.ts:
      • Used TRANSACTION_VERSION as the default value for transactionVersion in the estimateFees function.
    • executeTxn.ts:
      • Passed TRANSACTION_VERSION directly to a function.
    • upgradeAccContract.ts:
      • Added TRANSACTION_VERSION as a parameter.
    • constants.ts:
      • Defined TRANSACTION_VERSION and set it to constants.TRANSACTION_VERSION.V2.
    • starknetUtils.ts:
      • Utilized TRANSACTION_VERSION to provide default values in multiple functions.
  • Type Changes:

    • Updated transactionVersion type to accept either TRANSACTION_VERSION.V2 or TRANSACTION_VERSION.V3.
  • Function Signature Changes:

    • Functions now accept transactionVersion as an optional parameter with default values.

Impact:

  • Functional Impact:
    The changes do not alter current behavior. They set up default values and ensure consistent usage of TRANSACTION_VERSION across the codebase.

  • Preparatory Work:
    These changes lay the groundwork for handling different transaction versions in future updates.

  • Code Consistency:
    Ensures that TRANSACTION_VERSION is consistently used as a single source of truth across the codebase.

No changes to the transaction handling or fee estimation logic are introduced in this PR. This setup will facilitate easier integration of different transaction versions in future developments.

@khanti42 khanti42 requested a review from a team as a code owner June 27, 2024 05:51
@khanti42 khanti42 requested review from Akaryatrh and jonesho June 27, 2024 05:51
@khanti42 khanti42 changed the title feat: support txv3 in estimateFee and estimateFeeBulk feat: support txv3 in estimateFee and estimateFeeBulk [SNAP] Jul 2, 2024
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.

i will highly recommend to change to new api structure if it is possible

Copy link

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

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

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
63.6% Coverage on New Code
0.0% 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

@khanti42 khanti42 added this pull request to the merge queue Aug 15, 2024
Merged via the queue into main with commit d2dcbad Aug 15, 2024
13 checks passed
@khanti42 khanti42 deleted the feat/sf-602 branch August 15, 2024 10:34
@github-actions github-actions bot mentioned this pull request Aug 15, 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