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 block value to getPayload result #4884

Merged
merged 24 commits into from
Dec 5, 2022
Merged

Add block value to getPayload result #4884

merged 24 commits into from
Dec 5, 2022

Conversation

deffrian
Copy link
Contributor

@deffrian deffrian commented Nov 9, 2022

Fixes | Closes | Resolves #

Anything marked as optional that you didn't need to fill in, please remove it from the PR description. Choose one of the keywords above to refer to the issue this PR solves, followed by the issue number (e.g Fixes # 666). If there is no issue, remove the line. Remove this note after reading.

Changes:

Types of changes

What types of changes does your code introduce?
Put an x in the boxes that apply

  • Bugfix (non-breaking change which fixes an issue)
  • [ x ] New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation Update
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • Other (please describe):

Testing

Requires testing

  • [ x ] Yes
  • No

In case you checked yes, did you write tests??

  • [ x ] Yes
  • No

@deffrian deffrian linked an issue Nov 9, 2022 that may be closed by this pull request
Engine API: Add block value in response to engine_getPayload #4809
Closed
@deffrian deffrian marked this pull request as ready for review November 9, 2022 13:37
@deffrian deffrian changed the title Add block value in getPayload result Add block value to getPayload result Nov 10, 2022
@rubo
Copy link
Contributor

rubo commented Nov 14, 2022

@MarekM25 This gonna conflict hard with #4731

@MarekM25
Copy link
Contributor

@MarekM25 This gonna conflict hard with #4731

Yes, could you propose the solution?

@deffrian
Copy link
Contributor Author

I can merge this pr to #4731 after merging it into master

@rubo
Copy link
Contributor

rubo commented Nov 15, 2022 •
edited
Loading

I can merge this pr to #4731 after merging it into master

With this said, I'm making this draft to avoid an accidental merge.

@rubo rubo marked this pull request as draft November 15, 2022 17:05
@LukaszRozmej LukaszRozmej marked this pull request as ready for review November 16, 2022 10:53
LukaszRozmej
LukaszRozmej approved these changes Nov 18, 2022
View reviewed changes
Copy link
Member

@LukaszRozmej LukaszRozmej left a comment

Choose a reason for hiding this comment

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

the Fee value propagation in BlockHeader is suboptimal, but I don't have good solution of top of my head, maybe someone else has?

@deffrian
Fix encoding?
48e1102
@deffrian
Fix encoding?
306abc4
@deffrian
Revert commits
e9bcf53
@deffrian
Fix encoding
d72a0fd
@deffrian
Fix tests
bb5f753
@deffrian
Merge branch 'master' into feature/fees_in_getPayload
289573e 
# Conflicts:
#	src/Nethermind/Nethermind.Core/BlockHeader.cs
#	src/Nethermind/Nethermind.Evm/Tracing/BlockReceiptsTracer.cs
#	src/Nethermind/Nethermind.Evm/Tracing/CancellationTxTracer.cs
#	src/Nethermind/Nethermind.Evm/Tracing/CompositeTxTracer.cs
#	src/Nethermind/Nethermind.Evm/Tracing/ITxTracer.cs
#	src/Nethermind/Nethermind.Merge.Plugin/BlockProduction/IPayloadPreparationService.cs
#	src/Nethermind/Nethermind.Merge.Plugin/IEngineRpcModule.cs
#	src/Nethermind/Nethermind.Specs.Test/OverridableReleaseSpec.cs
@deffrian
Fix copyright
ede813d
MarekM25
MarekM25 reviewed Nov 23, 2022
View reviewed changes
Copy link
Contributor

@MarekM25 MarekM25 left a comment

Choose a reason for hiding this comment

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

Added a few comments and questions

MarekM25
MarekM25 reviewed Nov 28, 2022
View reviewed changes
Copy link
Contributor

@MarekM25 MarekM25 left a comment

Choose a reason for hiding this comment

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

One comment about free transactions.

QQ: How was the PR tested? (besides unit tests, of course :) )

@MarekM25 MarekM25 merged commit 54876b7 into master Dec 5, 2022
@MarekM25 MarekM25 deleted the feature/fees_in_getPayload branch December 5, 2022 18:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@LukaszRozmej LukaszRozmej LukaszRozmej approved these changes

@MarekM25 MarekM25 MarekM25 approved these changes

@tkstanczak tkstanczak Awaiting requested review from tkstanczak

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Engine API: Add block value in response to engine_getPayload
5 participants
@deffrian @rubo @MarekM25 @tkstanczak @LukaszRozmej