-
Notifications
You must be signed in to change notification settings - Fork 16
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
Add TransactionVersion
enum
#413
Conversation
* Standard versions include [V0], [V1], [V2], and [V3]. These are utilized for regular transaction execution. | ||
* Query versions [V1_QUERY], [V2_QUERY], and [V3_QUERY] should only be used for creating transactions to be used | ||
* in queries that do not alter the chain state, as in methods like [Provider.simulateTransactions] and [Provider.getEstimateFee]. | ||
* Avoid using query versions for transactions intended for execution. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Avoid using query versions for transactions intended for execution.
I'm pretty sure this should not even be possible, and such tx would fail
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not, hence the warning 😅
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But the warning says "Avoid using query versions for transactions intended for execution.", which implies it is possible, but should be avoided :P
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Which should convey to the user "it's possible from the SDK-side, but should not be done".
Should I rephrase it to:
Sending transaction with a query version for execution will result in a failure.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, explicit is better than implicit 👍
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #413 +/- ##
==========================================
- Coverage 69.30% 69.23% -0.08%
==========================================
Files 73 73
Lines 3131 3130 -1
Branches 315 308 -7
==========================================
- Hits 2170 2167 -3
- Misses 811 813 +2
Partials 150 150 ☔ View full report in Codecov by Sentry. |
Describe your changes
Related to #409
TransactionVersion
enumFelt
in tx, tx payload dataclasses and related functionsLinked issues
Closes #412
Breaking changes
version
argument inTransactionFactory
functions is now of typeTransactionVersion
instead ofFelt
version
argument inTransactionHashCalculator
functions is now of typeTransactionVersion
instead ofFelt
TransactionPayload.version
is now of typeTransactionVersion
instead ofFelt