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

Update charge fee and add n_steps for reverted transactions #787

Merged
merged 16 commits into from
Jul 17, 2023
Merged

Update charge fee and add n_steps for reverted transactions #787

merged 16 commits into from
Jul 17, 2023

Conversation

matias-gonz
Copy link
Contributor

Closes #716

@matias-gonz matias-gonz self-assigned this Jul 7, 2023
@codecov-commenter
Copy link

codecov-commenter commented Jul 7, 2023 •
edited
Loading

Codecov Report

Merging #787 (0ca2a2e) into main (8809ba0) will increase coverage by 0.10%.
The diff coverage is 98.97%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #787      +/-   ##
==========================================
+ Coverage   91.66%   91.76%   +0.10%     
==========================================
  Files          54       54              
  Lines       12834    12871      +37     
==========================================
+ Hits        11764    11811      +47     
+ Misses       1070     1060      -10
Impacted Files Coverage Δ
src/transaction/declare_v2.rs 83.78% <91.30%> (-0.43%) ⬇️
src/lib.rs 98.71% <100.00%> (-0.02%) ⬇️
src/testing/state.rs 94.28% <100.00%> (+0.01%) ⬆️
src/transaction/declare.rs 93.65% <100.00%> (-0.01%) ⬇️
src/transaction/deploy.rs 90.60% <100.00%> (+0.08%) ⬆️
src/transaction/deploy_account.rs 89.18% <100.00%> (-0.22%) ⬇️
src/transaction/fee.rs 95.49% <100.00%> (-3.32%) ⬇️
src/transaction/invoke_function.rs 99.47% <100.00%> (+0.14%) ⬆️
src/transaction/l1_handler.rs 97.15% <100.00%> (-0.03%) ⬇️
src/utils.rs 95.05% <100.00%> (+0.16%) ⬆️

... and 1 file with indirect coverage changes

juanbono
juanbono suggested changes Jul 12, 2023
View reviewed 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.

Lets wait to have the other PR for reverted tx ready before merging this one

@matias-gonz matias-gonz mentioned this pull request Jul 14, 2023
Merged
@matias-gonz
Copy link
Contributor Author

Merge #813 first

@matias-gonz matias-gonz added 0.12.1 0.12.1 Release and removed DO-NOT-MERGE labels Jul 17, 2023
MegaRedHand
MegaRedHand approved these changes Jul 17, 2023
View reviewed changes
Copy link
Contributor

@MegaRedHand MegaRedHand left a comment

Choose a reason for hiding this comment

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

LGTM

src/transaction/declare_v2.rs Show resolved Hide resolved
MegaRedHand
MegaRedHand reviewed Jul 17, 2023
View reviewed changes
src/transaction/fee.rs Outdated Show resolved Hide resolved
@juanbono @MegaRedHand
apply doc suggestion to src/transaction/fee.rs
cd9c918 
Co-authored-by: Tomás <47506558+MegaRedHand@users.noreply.github.com>
juanbono
juanbono reviewed Jul 17, 2023
View reviewed changes
src/transaction/fee.rs Outdated
/// # Parameters:
/// - `state`: A state that implements the [`State`] and [`StateReader`] traits.
/// - `resources`: the resources that are in use by the contract
/// - `block_context`: The block that contains the execution context
Copy link
Collaborator

Choose a reason for hiding this comment

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

the Parameters section looks incomplete. Also the block context description doesn't look accurate

@matias-gonz matias-gonz enabled auto-merge July 17, 2023 21:09
@matias-gonz matias-gonz added this pull request to the merge queue Jul 17, 2023
Merged via the queue into main with commit 5448721 Jul 17, 2023
@matias-gonz matias-gonz deleted the price_reverted_transactions branch July 18, 2023 14:23
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@MegaRedHand MegaRedHand MegaRedHand approved these changes

@SantiagoPittella SantiagoPittella SantiagoPittella approved these changes

@juanbono juanbono juanbono approved these changes

Assignees

@matias-gonz matias-gonz

Labels
0.12.1 0.12.1 Release
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Correctly price reverted transactions
5 participants
@matias-gonz @codecov-commenter @juanbono @MegaRedHand @SantiagoPittella