-
Notifications
You must be signed in to change notification settings - Fork 441
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
Introduce ExecutionPayloadParams concept to be able to better compose… #5932
Introduce ExecutionPayloadParams concept to be able to better compose… #5932
Conversation
… and validate complex method signatures
…nly' into feature/eip-4844-v3-handles-v3-only-ExecutionPayloadParams
This reverts commit 21c72e4.
src/Nethermind/Nethermind.Merge.Plugin/EngineRpcModule.Paris.cs
Outdated
Show resolved
Hide resolved
@@ -43,7 +43,7 @@ public async Task NewPayloadV1_should_decline_post_cancun() | |||
|
|||
ResultWrapper<PayloadStatusV1> errorCode = (await rpcModule.engine_newPayloadV1(executionPayload)); | |||
|
|||
Assert.That(errorCode.ErrorCode, Is.EqualTo(ErrorCodes.InvalidParams)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assume that we should return InvalidParams still here
So if you pass newPayloadV1 with post-cancun timestamp your validation should fail once checking that BlobVersionedHashes are null and it should give you InvalidParams
UnsupportedFork will be when you pass newPayloadV3 with pre-cancun timestamp
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The idea is to show this error if we have Cancun timestamp in V2 and pre-Cancun timestamp in V3
… and validate complex method signatures
SImplify the code a bit.
Tests are failing but it is problem with tests, need to be fixed in main branch as they might get refactored.