Skip to content
This repository was archived by the owner on Jul 22, 2024. It is now read-only.

Add version query #810

Merged
merged 11 commits into from
Jul 15, 2023
Merged

Add version query #810

merged 11 commits into from
Jul 15, 2023

Conversation

mmsc2
Copy link
Contributor

@mmsc2 mmsc2 commented Jul 11, 2023

TITLE

Description

solves issue #799

@mmsc2 mmsc2 linked an issue Jul 11, 2023 that may be closed by this pull request
@codecov-commenter
Copy link

codecov-commenter commented Jul 11, 2023

Codecov Report

Merging #810 (917d90d) into main (7d9b7a6) will decrease coverage by 0.09%.
The diff coverage is 97.64%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #810      +/-   ##
==========================================
- Coverage   91.91%   91.82%   -0.09%     
==========================================
  Files          52       52              
  Lines       12275    12373      +98     
==========================================
+ Hits        11282    11361      +79     
- Misses        993     1012      +19     
Impacted Files Coverage Δ
src/transaction/declare_v2.rs 79.18% <96.72%> (+3.70%) ⬆️
src/execution/mod.rs 97.56% <100.00%> (+0.01%) ⬆️
src/transaction/declare.rs 93.75% <100.00%> (ø)
src/transaction/deploy_account.rs 89.70% <100.00%> (ø)
src/transaction/invoke_function.rs 99.33% <100.00%> (+<0.01%) ⬆️
src/transaction/verify_version.rs 100.00% <100.00%> (ø)

... and 3 files with indirect coverage changes

Copy link
Collaborator

@juanbono juanbono left a comment

Choose a reason for hiding this comment

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

wen tests?

@@ -14,6 +14,11 @@ pub(crate) const N_DEFAULT_TOPICS: usize = 1; // Events have one default topic.
pub(crate) const CONSUMED_MSG_TO_L2_ENCODED_DATA_SIZE: usize =
(L1_TO_L2_MSG_HEADER_SIZE + 1) - CONSUMED_MSG_TO_L2_N_TOPICS;

lazy_static! {
pub(crate) static ref VERSION_QUERY: Felt252 =
Copy link
Collaborator

Choose a reason for hiding this comment

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

the name in cairo-lang is VERSION_QUERY_BASE I think we should have the same name because VERSION_QUERY has an specific meaning in this context:

QUERY_VERSION = QUERY_VERSION_BASE + TRANSACTION_VERSION
Where TRANSACTION_VERSION = 1

Copy link
Collaborator

@SantiagoPittella SantiagoPittella left a comment

Choose a reason for hiding this comment

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

We could compare references instead of cloning.

@juanbono juanbono added this pull request to the merge queue Jul 15, 2023
Merged via the queue into main with commit 71e98dc Jul 15, 2023
@juanbono juanbono deleted the AddVersionQuery branch July 31, 2023 23:36
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Handle transaction versions with the QUERY_VERSION_BASE set
5 participants