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

Addition of Transient Storage Opcodes #2778

Merged
merged 38 commits into from
Dec 23, 2024

Conversation

fmacleal
Copy link
Contributor

@fmacleal fmacleal commented Oct 1, 2024

This PR aims to implements the RSKIP-446 that handles the transient storage opcodes defined int he RSKIP-446, which aims to be compatible with EIP-1153 from Ethereum.

The PR still in draft mode because some tests need to be fixed and added.

Description

Motivation and Context

How Has This Been Tested?

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • Tests for the changes have been added (for bug fixes / features)
  • Requires Activation Code (Hard Fork)
  • Other information:

@fmacleal fmacleal force-pushed the fmacleal/addition_transient_storage_opcodes branch from d6dc804 to 8892bf3 Compare October 14, 2024 10:46
Copy link

github-actions bot commented Oct 25, 2024

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

OpenSSF Scorecard

PackageVersionScoreDetails

Scanned Manifest Files

* Adding the structure for transient storage opcodes

- Finishing test validation for basic scenarios of TLOAD/TSTORE

* Refactor from code to get advantage from the MutableRepository

- In order to get advantage from the whole flow of tracking, rollback and commits already present in the MutableRepository.
We refactored a bit the logic of transient storage, now it's simpler.

* Adding more tests defined on the EIP-1153 spec

* Adding tests for the different create contexts EIP1153

* Addressing comments from review

* Adding more scenarios of test with dynamic execution context
@fmacleal fmacleal force-pushed the fmacleal/addition_transient_storage_opcodes branch from f244dc2 to 551df42 Compare November 4, 2024 14:06
@fmacleal fmacleal force-pushed the fmacleal/addition_transient_storage_opcodes branch from 612a66a to 99599fb Compare November 5, 2024 14:05
* Added tests for dynamic execution contexts

- WIP
- Now we will add unit tests for underflow execution tests

* Added tests for dynamic execution contexts

-  We had to add unit tests for underflow execution tests since isn't possible write contract for these scenarios
- Now we have all the tests regarding execution context with different types of calls, missing the gas
cost calculation that will be done in a later task

* Addressing review comments
//given
ActivationConfig.ForBlock activations = mock(ActivationConfig.ForBlock.class);
when(activations.isActive(RSKIP446)).thenReturn(true);
String expected = "0000000000000000000000000000000000000000000000000000000000000000";

Check notice

Code scanning / CodeQL

Unread local variable Note test

Variable 'String expected' is never read.
fmacleal and others added 5 commits December 3, 2024 09:02
- Added basic scenario of gas measure costs for Transient Storage
- Stil needed to add more scenarios of tests for run out of gas
- Added basic scenario of gas measure costs for Transient Storage
- Added scenarios of run out of gas exception loops
- Still missing some tests with gas consumption for some execution contexts
…ition

Transient Storage gas cost calculation
@fmacleal fmacleal marked this pull request as ready for review December 11, 2024 07:47
fmacleal and others added 3 commits December 16, 2024 12:56
- As specified in the EIP-1153, it was added test scenarios for reentrancy after
selfdestruct.
- Besides that, we refactored the folder that contained the DSL tests files
@donequis
Copy link
Collaborator

The transient storage resets between blocks, but not between transactions. It'd be good to add a test with multiple transactions on the same block on the test dsl tests.

@fmacleal fmacleal requested a review from Vovchyk December 17, 2024 06:54
@fmacleal fmacleal force-pushed the fmacleal/addition_transient_storage_opcodes branch from d0ccf35 to 2090bea Compare December 17, 2024 12:51
- After have added a scenario of DSL test where two we check storage between transactions inside same block, we noticed a bug. This commit fixes it clearing the transient storage after the tx finishes.
@fmacleal fmacleal requested a review from donequis December 18, 2024 15:04
@donequis
Copy link
Collaborator

This looks good now

- In order to have a better validation of real use of TLOAD/TSTORE, we changed some DSL tests to not create
a new block after every tx. This way, we validate that the transient storage it's correctly managed across the different
tx.

- Small refactor was added in the MutableTrie creation with a better name stating that we are creating an in memory
trie.
@Vovchyk Vovchyk merged commit 87bde23 into master Dec 23, 2024
13 checks passed
@Vovchyk Vovchyk deleted the fmacleal/addition_transient_storage_opcodes branch December 23, 2024 14:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants