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

Backwards compatibility fix on tx interceptor for gaslimit value #3624

Merged

Conversation

iulianpascalau
Copy link
Contributor

  • removed backwards compatibility issue on transactions at interceptor level
  • added protection on the API for transactions with higher gas limit than the transaction processor can cope

…r level

- added protection on the API for transactions with higher gas limit than the transaction processor can cope
@iulianpascalau iulianpascalau self-assigned this Dec 6, 2021
@iulianpascalau iulianpascalau added the type:bug Something isn't working label Dec 6, 2021
@codecov
Copy link

codecov bot commented Dec 6, 2021

Codecov Report

Merging #3624 (cc789ff) into development (9b0fcfa) will decrease coverage by 0.00%.
The diff coverage is 68.00%.

Impacted file tree graph

@@               Coverage Diff               @@
##           development    #3624      +/-   ##
===============================================
- Coverage        73.69%   73.69%   -0.01%     
===============================================
  Files              584      585       +1     
  Lines            75295    75317      +22     
===============================================
+ Hits             55489    55503      +14     
- Misses           15398    15405       +7     
- Partials          4408     4409       +1     
Impacted Files Coverage Δ
factory/coreComponentsHandler.go 57.03% <0.00%> (-1.30%) ⬇️
factory/coreComponents.go 80.09% <50.00%> (-0.62%) ⬇️
node/node.go 70.64% <100.00%> (ø)
process/economics/apiEconomicsData.go 100.00% <100.00%> (ø)
process/economics/economicsData.go 72.26% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cb5e5e2...cc789ff. Read the comment docs.

SebastianMarian
SebastianMarian previously approved these changes Dec 6, 2021
}, nil
}

// CheckValidityTxValues checks if the provided transaction is economically correct. It overloads the original method
Copy link
Contributor

Choose a reason for hiding this comment

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

if the provided transaction is economically correct => if the provided transaction respects the gas limit restrictions

Copy link
Contributor Author

Choose a reason for hiding this comment

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

actually it also checks other stuff other than the gas limit as it calls the overloaded method

)

func TestNewAPIEconomicsData(t *testing.T) {
t.Parallel()
Copy link
Contributor

Choose a reason for hiding this comment

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

t.Parallel on each inner test, as right now the inner tests will be run serial

Copy link
Contributor Author

Choose a reason for hiding this comment

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

please check. Can not be done.

economicsData, _ := economics.NewEconomicsData(args)
apiEconomicsData, _ := economics.NewAPIEconomicsData(economicsData)

t.Run("maximum gas limit as defined should work", func(t *testing.T) {
Copy link
Contributor

Choose a reason for hiding this comment

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

t.Parallel() on each inner test

Copy link
Contributor Author

Choose a reason for hiding this comment

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

please check. Can not be done.
Done

Copy link
Collaborator

@gabi-vuls gabi-vuls left a comment

Choose a reason for hiding this comment

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

System test passed.
Errors are safe and will be solved soon.
image

@gabi-vuls gabi-vuls merged commit 2882926 into development Dec 8, 2021
@gabi-vuls gabi-vuls deleted the backwards-compatibility-gaslimit-tx-interceptor branch December 8, 2021 12:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants