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.

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

SKALED-1745 Fixed version of corrent fork in checkOutExternalGas #1782

Merged
merged 4 commits into from
Jan 12, 2024

Conversation

dimalit
Copy link
Contributor

@dimalit dimalit commented Jan 11, 2024

Changes:
1 Transaction::checkOutExternalGas() now operates on current Ethereum fork, instead of Constantinople (base gas for 1 byte of data changed from 68 gas/b to 16 gas/b)
2 All changes are activated after correctForkInPowPatchTimestamp. This is true both for external gas check and for eth_estimateGas call.
3 Transaction::checkOutExternalGas() is called one additional time after consensus (needed to be sure that patch activates smoothly). This make possible little decrease of block processing performance (but unlikely).

Testing:
JsonRpcSuite/simplePoWTransaction tests that PoW transaction with incorrect amount of gas fails before patch timestamp and works after it

UPD: This is 2nd PR - after fixing functional tests

UPD2: For some reason, on this version fails functional test load_js/cat_cycle. It works on 3.18.0-develop.22, but fails on this branch. During test, skaled doesn't crash or exit, but transactions sending script at first works for some time, but then fails with "cannot connect" error.

…pow-gas' into bug/SKALED-1745-invalid-fork-in-pow-gas
@dimalit dimalit changed the title SKALED-1745 Init patch timestamp in Client::init (to prevent failure) SKALED-1745 Fixed version of corrent fork in checkOutExternalGas Jan 11, 2024
Copy link

codecov bot commented Jan 11, 2024

Codecov Report

Attention: 4 lines in your changes are missing coverage. Please review.

Comparison is base (bba2a8a) 45.54% compared to head (d026bcb) 45.60%.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           v3.18.0    #1782      +/-   ##
===========================================
+ Coverage    45.54%   45.60%   +0.06%     
===========================================
  Files          356      358       +2     
  Lines        51766    51820      +54     
===========================================
+ Hits         23577    23635      +58     
+ Misses       28189    28185       -4     

@dimalit dimalit linked an issue Jan 11, 2024 that may be closed by this pull request
@olehnikolaiev olehnikolaiev self-requested a review January 11, 2024 16:08
@kladkogex kladkogex merged commit fc5b582 into v3.18.0 Jan 12, 2024
8 checks passed
@kladkogex kladkogex deleted the bug/SKALED-1745-invalid-fork-in-pow-gas branch January 12, 2024 18:47
@github-actions github-actions bot locked and limited conversation to collaborators Jan 12, 2024
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.

Invalid gas computation/fork block in checkOutEternalGas
3 participants