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

Initial constants and config for the max initcode size #2627

Merged
merged 7 commits into from
Sep 19, 2024

Conversation

fmacleal
Copy link
Contributor

In order to implement the verification of initcode size, necessary for the RSKIP438, we need the constant value with the max allowed value. So we can start to validate and use it on the logic of the verification

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 feature/introduce_initcode_size_limit branch 4 times, most recently from 8969bd9 to 3fd6e2d Compare July 22, 2024 13:45
@fmacleal fmacleal force-pushed the feature/introduce_initcode_size_limit branch from 3fd6e2d to 8b74286 Compare August 2, 2024 10:29
asoto-iov
asoto-iov previously approved these changes Aug 7, 2024
Vovchyk
Vovchyk previously approved these changes Aug 7, 2024
@fmacleal fmacleal force-pushed the feature/introduce_initcode_size_limit branch from 8b74286 to 482204c Compare August 12, 2024 12:21
@fmacleal fmacleal force-pushed the feature/introduce_initcode_size_limit branch from 482204c to 59943d6 Compare August 23, 2024 17:19
@fmacleal fmacleal marked this pull request as ready for review August 26, 2024 14:05
nagarev
nagarev previously approved these changes Aug 26, 2024
Copy link
Contributor

@nagarev nagarev left a comment

Choose a reason for hiding this comment

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

LGTM! :shipit:

@nagarev
Copy link
Contributor

nagarev commented Aug 29, 2024

pipeline:run

@fmacleal fmacleal force-pushed the feature/introduce_initcode_size_limit branch from 59943d6 to 319d678 Compare September 5, 2024 14:06
In order to implement the verification of initcode size, necessary for the RSKIP438,
we need the constant value with the max allowed value. So we can start to validate
and use it on the logic of the verification
@fmacleal fmacleal force-pushed the feature/introduce_initcode_size_limit branch from 319d678 to f5da98a Compare September 5, 2024 14:13
@fmacleal fmacleal dismissed stale reviews from asoto-iov, nagarev, and Vovchyk via 236401e September 8, 2024 13:42
* Initial constants and config for the max initcode size

In order to implement the verification of initcode size, necessary for the RSKIP438,
we need the constant value with the max allowed value. So we can start to validate
and use it on the logic of the verification

- Adding scenarios of test for the contract creation with DSL
- The validation for the initCode size test was added
- The validation is based on the RSKIP that enables it
- Validation added for the contractCreation during transaction execution

Finished the implementation of transactionCost and initcode size validation

- All the logic related with transaction cost was changed to meet the new cost criteria
- More tests were added to be sure that the flow is following the RSKIP438
- Tests were added to validate that this cost and validation doesn't impact before reach the activation height

Applied some suggestions from review

Adding tests to validate the failure if we don't have enough gas to cover initCode

* Refactor from the Initcode cost calculation

In order to have the exact same logic for the initcode cost calculation,
we introduced some new classes that can be easily reused by the TransactionExecutor and VM classes. This way, if this logic change, we won't have to change the code in multiple places.

It was also introduced an interface for it, so we can expand this idea and
in the future, be able to calculate total transactions just calling a method
to calculate it independent of the logic used.

* Applying some more suggestions from review

Co-Authored-By: Nazaret García Revetria <[email protected]>
Co-Authored-By: Vovchyk <[email protected]>
@fmacleal fmacleal force-pushed the feature/introduce_initcode_size_limit branch from 236401e to 6a0e6e1 Compare September 9, 2024 15:11
fmacleal and others added 2 commits September 9, 2024 17:45
* Initial constants and config for the max initcode size

In order to implement the verification of initcode size, necessary for the RSKIP438,
we need the constant value with the max allowed value. So we can start to validate
and use it on the logic of the verification

- Adding scenarios of test for the contract creation with DSL
- The validation for the initCode size test was added
- The validation is based on the RSKIP that enables it
- Validation added for the contractCreation during transaction execution

Finished the implementation of transactionCost and initcode size validation

- All the logic related with transaction cost was changed to meet the new cost criteria
- More tests were added to be sure that the flow is following the RSKIP438
- Tests were added to validate that this cost and validation doesn't impact before reach the activation height

Applied some suggestions from review

Adding tests to validate the failure if we don't have enough gas to cover initCode

* Refactor from the Initcode cost calculation

In order to have the exact same logic for the initcode cost calculation,
we introduced some new classes that can be easily reused by the TransactionExecutor and VM classes. This way, if this logic change, we won't have to change the code in multiple places.

It was also introduced an interface for it, so we can expand this idea and
in the future, be able to calculate total transactions just calling a method
to calculate it independent of the logic used.

* Applying some more suggestions from review

Co-Authored-By: Nazaret García Revetria <[email protected]>
Co-Authored-By: Vovchyk <[email protected]>
…ksmart/rskj into feature/introduce_initcode_size_limit
@Vovchyk
Copy link
Contributor

Vovchyk commented Sep 9, 2024

pipeline:run

Vovchyk
Vovchyk previously approved these changes Sep 9, 2024
Copy link

sonarcloud bot commented Sep 19, 2024

@Vovchyk Vovchyk merged commit 6fe81bb into master Sep 19, 2024
10 checks passed
@Vovchyk Vovchyk deleted the feature/introduce_initcode_size_limit branch September 19, 2024 14:22
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