Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Account.deploy_account_v3
static method #1265Add
Account.deploy_account_v3
static method #1265Changes from 7 commits
0678c7d
62ff260
6deca72
26433ba
5039311
eacdbc1
f792958
a52b796
d14e1fa
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
I am not sure how to implement a similar check for the V3 transaction now. I'm guessing eventually we'd need to pull the balance from the STRK contract and an exchange rate to WEI?
Let me know if I am missing something here. Generally I think it is not crucial, but definitely nice to have.
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.
Good question. I suppose it could depend on fee DA mode as well, but I'm not 100% sure. Let's open an issue to investigate this, we might need to ask SW as well.
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.
#1266
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.
This is not crucial since tx will be rejected with a descriptive enough error message anyway
It should probably be done the same way, except we'll have to pass STRK token address to
get_balance
. I don't really understand why'd you want to convert to WEI though?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.
You're absolutely right, there is no need for conversion. I thought that
L1_GAS
in the resource bounds is specified in WEI, even though V3 transaction costs are paid in FRI, which is not the case. Apologies for the confusion.We can indeed check it the same way as for V1, but with the STRK contract. On the other hand, as @DelevoXDG said, it is not crucial, since it will throw an error anyway. @THenry14 Wdyt?
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.
Agreed, let's update #1266 with the findings and do necessary changes under that issue at some later date