Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add gasConsumed cases for Ethereum transactions #8207

Merged
merged 12 commits into from
May 30, 2024
Merged

Add gasConsumed cases for Ethereum transactions #8207

merged 12 commits into from
May 30, 2024

Conversation

vtronkov
Copy link
Contributor

Description:

  • Adding gasConsumed tests

Related issue(s):

Fixes hashgraph/hedera-services#12948

Notes for reviewer:

  • Moving the verifyGasConsumed logic into AbstractEstimateFeature.java to reuse the logic
  • Using the verifyGasConsumed to verify the correct gasConsumed

Checklist

  • Documented (Code comments, README, etc.)
  • Tested (unit, integration, etc.)

JivkoKelchev and others added 3 commits April 18, 2024 16:28
@JivkoKelchev
Add ethereum transactions scenario to the acceptance tests
ab5209e 
Signed-off-by: Zhivko Kelchev <zhivko.kelchev@limechain.tech>
@JivkoKelchev
upgrade bouncycastle
ef65fc1 
Signed-off-by: Zhivko Kelchev <zhivko.kelchev@limechain.tech>
@vtronkov
test: add gasConsumed tests
afa7974 
Signed-off-by: Valentin Tronkov <99957253+vtronkov@users.noreply.github.com>
@vtronkov vtronkov self-assigned this Apr 30, 2024
@vtronkov vtronkov added the limechain Work planned for the LimeChain team label Apr 30, 2024
@vtronkov vtronkov marked this pull request as ready for review April 30, 2024 11:31
@vtronkov vtronkov changed the title test: add gasConsumed tests test: Add gasConsumed cases for Ethereum transactions Apr 30, 2024
@vtronkov
refactor: remove txId from DeployedContract
27dd57c 
Signed-off-by: Valentin Tronkov <99957253+vtronkov@users.noreply.github.com>
@vtronkov
refactor: separate the gasVerify check into a separate step
6381c49 
Signed-off-by: Valentin Tronkov <99957253+vtronkov@users.noreply.github.com>
@vtronkov
refactor: move gasConsumedSelector in the abstract class
1b96c70 
Signed-off-by: Valentin Tronkov <99957253+vtronkov@users.noreply.github.com>
@vtronkov
refactor: rename the verify gasConsumed step
87cc090 
Signed-off-by: Valentin Tronkov <99957253+vtronkov@users.noreply.github.com>
steven-sheehy
steven-sheehy previously approved these changes May 10, 2024
View reviewed changes
Copy link
Member

@steven-sheehy steven-sheehy left a comment

Choose a reason for hiding this comment

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

LGTM. I'll leave it up to you to merge

@vtronkov
Copy link
Contributor Author

LGTM. I'll leave it up to you to merge

Thank you. Since it's branched off 7655-EthereumTransaction-tests I will wait for it to be merged first and then I will merge this one.

Base automatically changed from 7655-EthereumTransaction-tests to main May 29, 2024 22:41
@steven-sheehy steven-sheehy dismissed their stale review May 29, 2024 22:41

The base branch was changed.

@steven-sheehy steven-sheehy added this to the 0.107.0 milestone May 29, 2024
@steven-sheehy
Copy link
Member

Needs a merge from main

@vtronkov
Merge branch 'main' into 12948-add-gasConsumed-cases-for-Ethereum-tra…
1153fad 
…nsactions

# Conflicts:
#	hedera-mirror-test/src/test/java/com/hedera/mirror/test/e2e/acceptance/client/EthereumClient.java
#	hedera-mirror-test/src/test/java/com/hedera/mirror/test/e2e/acceptance/util/ethereum/EthTxData.java
#	hedera-mirror-test/src/test/java/com/hedera/mirror/test/e2e/acceptance/util/ethereum/EthTxSigs.java
@vtronkov
fix: merge conflicts
5876ed1 
Signed-off-by: Valentin Tronkov <99957253+vtronkov@users.noreply.github.com>
@vtronkov
fix: merge conflicts
00ca84c 
Signed-off-by: Valentin Tronkov <99957253+vtronkov@users.noreply.github.com>
@vtronkov
nit: remove not used fields
88e58ca 
Signed-off-by: Valentin Tronkov <99957253+vtronkov@users.noreply.github.com>
@codecov Codecov
Copy link

codecov bot commented May 30, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.50%. Comparing base (ffb5cfa) to head (8757a96).
Report is 1 commits behind head on main.

Additional details and impacted files 
@@             Coverage Diff              @@
##               main    #8207      +/-   ##
============================================
+ Coverage     92.36%   92.50%   +0.14%     
+ Complexity     7376     7267     -109     
============================================
  Files           902      884      -18     
  Lines         29504    29230     -274     
  Branches       3603     3580      -23     
============================================
- Hits          27250    27040     -210     
+ Misses         1438     1379      -59     
+ Partials        816      811       -5

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@steven-sheehy steven-sheehy added enhancement Type: New feature test Test infrastructure, automated tests required, etc labels May 30, 2024
@vtronkov
refactor: address PR comments
8757a96 
Signed-off-by: Valentin Tronkov <99957253+vtronkov@users.noreply.github.com>
@sonarcloud SonarCloud
Copy link

sonarcloud bot commented May 30, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

steven-sheehy
steven-sheehy approved these changes May 30, 2024
View reviewed changes
Copy link
Member

@steven-sheehy steven-sheehy left a comment

Choose a reason for hiding this comment

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

LGTM

@steven-sheehy steven-sheehy changed the title test: Add gasConsumed cases for Ethereum transactions Add gasConsumed cases for Ethereum transactions May 30, 2024
@steven-sheehy steven-sheehy merged commit 8b10694 into main May 30, 2024
29 checks passed
@steven-sheehy steven-sheehy deleted the 12948-add-gasConsumed-cases-for-Ethereum-transactions branch May 30, 2024 20:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@steven-sheehy steven-sheehy steven-sheehy approved these changes

Assignees

@vtronkov vtronkov

Labels
enhancement Type: New feature limechain Work planned for the LimeChain team test Test infrastructure, automated tests required, etc
Projects
Smart Contract Sprint Board
Archived in project
Milestone
0.107.0
Development

Successfully merging this pull request may close these issues.

Add gasConsumed cases for Ethereum transactions
3 participants
@vtronkov @steven-sheehy @JivkoKelchev