-
Notifications
You must be signed in to change notification settings - Fork 27
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
test: streamline the v0 version tx for invoke tx towards code dedup in test utils #747
Conversation
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. Join @ArniStarkware and the rest of your teammates on Graphite |
Benchmark movements: |
73ecaad
to
baf5737
Compare
7d480c2
to
e34425f
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## arni/test_utils/deploy_account/blockifier #747 +/- ##
==============================================================================
+ Coverage 49.00% 76.61% +27.61%
==============================================================================
Files 195 102 -93
Lines 23300 13677 -9623
Branches 23300 13677 -9623
==============================================================================
- Hits 11418 10479 -939
+ Misses 11367 2742 -8625
+ Partials 515 456 -59 ☔ View full report in Codecov by Sentry. |
baf5737
to
51c315a
Compare
51c315a
to
e36c0d0
Compare
Benchmark movements: |
c569fec
to
bf37c40
Compare
e36c0d0
to
ce0392e
Compare
Benchmark movements: |
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.
Reviewable status: 0 of 2 files reviewed, 1 unresolved discussion (waiting on @ArniStarkware)
a discussion (no related file):
Can you please explain the change here?
Why does V0 have a special treatment, and why is it no longer necessary?
bf37c40
to
7c12924
Compare
ce0392e
to
1608275
Compare
1608275
to
7a04c94
Compare
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.
Reviewable status: 0 of 2 files reviewed, 1 unresolved discussion (waiting on @MohammadNassar1)
a discussion (no related file):
For version 0 transactions, the field entry_point_selector
must always be a specific thing:
/// V0 transactions should always select the
__execute__
entry point.
/// The original code from the blockifier crate was:
To compute the entry point selector value, you must use functions specifically from blockifier::abi::abi_utils
, namely: selector_from_name
. In this PR, we compute the value manually, offline, and hard code it in SNAPI.
The question becomes do we want to avoid this hard coding of a value.
The other thing we can do is move the abi
utils into snapi.
Stuff like moving the function starknet_keccak
into snapi - which might be fair game....
a2e08cb
to
0c39a59
Compare
Use lazylock Code quote: static EXECUTE_ENTRY_POINT_SELECTOR: OnceLock<EntryPointSelector> = OnceLock::new(); |
bd7724b
to
f74d1b4
Compare
0c39a59
to
f25ee2e
Compare
Artifacts upload triggered. View details here |
f74d1b4
to
35f5ff7
Compare
f25ee2e
to
8c57193
Compare
8c57193
to
9557b39
Compare
Artifacts upload triggered. View details here |
Artifacts upload triggered. View details here |
Artifacts upload triggered. View details here |
Artifacts upload triggered. View details here |
Artifacts upload triggered. View details here |
Benchmark movements: |
9557b39
to
0e25403
Compare
Artifacts upload triggered. View details here |
0e25403
to
fd8a886
Compare
Artifacts upload triggered. View details here |
Artifacts upload triggered. View details here |
This change is