Skip to content

Commit

Permalink
Improve return codes (UnsupportedFork) (#6029)
Browse files Browse the repository at this point in the history
  • Loading branch information
@flcl42
flcl42 authored Aug 23, 2023
1 parent ccb6238 commit d2dcf5f
Show file tree
Hide file tree
Showing 3 changed files with 143 additions and 28 deletions.
79 changes: 56 additions & 23 deletions src/Nethermind/Nethermind.Consensus/Producers/PayloadAttributes.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,8 @@ public string ToString(string indentation)
}
}

public enum PayloadAttributesValidationResult : byte { Success, InvalidParams, UnsupportedFork };

public static class PayloadAttributesExtensions
{
public static string ComputePayloadId(this PayloadAttributes payloadAttributes, BlockHeader parentHeader)
Expand Down Expand Up @@ -100,38 +102,69 @@ public static int GetVersion(this PayloadAttributes executionPayload) =>
public static int ExpectedEngineSpecVersion(this IReleaseSpec spec) =>
spec switch
{
{ WithdrawalsEnabled: true, IsEip4844Enabled: true } => EngineApiVersions.Cancun,
{ IsEip4844Enabled: true } => EngineApiVersions.Cancun,
{ WithdrawalsEnabled: true } => EngineApiVersions.Shanghai,
_ => EngineApiVersions.Paris
};

public static bool Validate(
this PayloadAttributes payloadAttributes,
IReleaseSpec spec,
int version,
public static PayloadAttributesValidationResult Validate(
this PayloadAttributes payloadAttributes,
ISpecProvider specProvider,
int apiVersion,
[NotNullWhen(false)] out string? error) =>
Validate(
apiVersion: apiVersion,
actualVersion: payloadAttributes.GetVersion(),
expectedVersion: specProvider.GetSpec(ForkActivation.TimestampOnly(payloadAttributes.Timestamp))
.ExpectedEngineSpecVersion(),
"PayloadAttributesV",
out error);

public static PayloadAttributesValidationResult Validate(
int apiVersion,
int actualVersion,
int expectedVersion,
string methodName,
[NotNullWhen(false)] out string? error)
{
int actualVersion = payloadAttributes.GetVersion();
int expectedVersion = spec.ExpectedEngineSpecVersion();

error = null;
if (actualVersion != expectedVersion)
if (apiVersion >= EngineApiVersions.Cancun)
{
error = $"PayloadAttributesV{expectedVersion} expected";
if (actualVersion == apiVersion && expectedVersion != apiVersion)
{
error = $"{methodName}{expectedVersion} expected";
return PayloadAttributesValidationResult.UnsupportedFork;
}
}
else if (actualVersion > version)
else if (apiVersion == EngineApiVersions.Shanghai)
{
error = $"PayloadAttributesV{version} expected";
if (actualVersion == apiVersion && expectedVersion >= EngineApiVersions.Cancun)
{
error = $"{methodName}{expectedVersion} expected";
return PayloadAttributesValidationResult.UnsupportedFork;
}
}
return error is null;
}

public static bool Validate(this PayloadAttributes payloadAttributes,
ISpecProvider specProvider,
int version,
[NotNullWhen(false)] out string? error) =>
payloadAttributes.Validate(
specProvider.GetSpec(ForkActivation.TimestampOnly(payloadAttributes.Timestamp)),
version,
out error);
if (actualVersion == expectedVersion)
{
if (apiVersion >= EngineApiVersions.Cancun)
{
if (actualVersion == apiVersion)
{
error = null;
return PayloadAttributesValidationResult.Success;
}
}
else
{
if (apiVersion >= actualVersion)
{
error = null;
return PayloadAttributesValidationResult.Success;
}
}
}

error = $"{methodName}{expectedVersion} expected";
return PayloadAttributesValidationResult.InvalidParams;
}
}
83 changes: 81 additions & 2 deletions src/Nethermind/Nethermind.Merge.Plugin.Test/EngineModuleTests.V3.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ public async Task GetPayloadV3_should_fail_on_unknown_payload()
public async Task GetPayloadV3_should_return_all_the_blobs(int blobTxCount)
{
(IEngineRpcModule rpcModule, string payloadId, _) = await BuildAndGetPayloadV3Result(Cancun.Instance, blobTxCount);
var result = await rpcModule.engine_getPayloadV3(Bytes.FromHexString(payloadId));
ResultWrapper<GetPayloadV3Result?> result = await rpcModule.engine_getPayloadV3(Bytes.FromHexString(payloadId));
BlobsBundleV1 getPayloadResultBlobsBundle = result.Data!.BlobsBundle!;
Assert.That(result.Data.ExecutionPayload.BlobGasUsed, Is.EqualTo(BlobGasCalculator.CalculateBlobGas(blobTxCount)));
Assert.That(getPayloadResultBlobsBundle.Blobs!.Length, Is.EqualTo(blobTxCount));
Expand Down Expand Up @@ -163,7 +163,6 @@ public virtual async Task NewPayloadV3_should_decline_mempool_encoding(bool inMe
[Test]
public async Task NewPayloadV3_should_decline_null_blobversionedhashes()
{

(JsonRpcService jsonRpcService, JsonRpcContext context, EthereumJsonSerializer serializer, ExecutionPayloadV3 executionPayload)
= await PreparePayloadRequestEnv();

Expand Down Expand Up @@ -239,6 +238,31 @@ public async Task NewPayloadV3_should_decline_empty_fields()
}
}

[TestCaseSource(nameof(ForkchoiceUpdatedV3DeclinedTestCaseSource))]
[TestCaseSource(nameof(ForkchoiceUpdatedV3AcceptedTestCaseSource))]

public async Task<int> ForkChoiceUpdated_should_return_proper_error_code(IReleaseSpec releaseSpec, string method, bool isBeaconRootSet)
{
MergeTestBlockchain chain = await CreateBlockchain(releaseSpec: releaseSpec);
IEngineRpcModule rpcModule = CreateEngineModule(chain);
ForkchoiceStateV1 fcuState = new(Keccak.Zero, Keccak.Zero, Keccak.Zero);
PayloadAttributes payloadAttributes = new()
{
Timestamp = chain.BlockTree.Head!.Timestamp,
PrevRandao = Keccak.Zero,
SuggestedFeeRecipient = Address.Zero,
Withdrawals = new List<Withdrawal>(),
ParentBeaconBlockRoot = isBeaconRootSet ? Keccak.Zero : null,
};

string response = await RpcTest.TestSerializedRequest(rpcModule, method,
chain.JsonSerializer.Serialize(fcuState),
chain.JsonSerializer.Serialize(payloadAttributes));
JsonRpcErrorResponse errorResponse = chain.JsonSerializer.Deserialize<JsonRpcErrorResponse>(response);

return errorResponse.Error?.Code ?? ErrorCodes.None;
}

private const string FurtherValidationStatus = "FurtherValidation";

[TestCaseSource(nameof(BlobVersionedHashesMatchTestSource))]
Expand Down Expand Up @@ -327,6 +351,61 @@ public async Task<string> NewPayloadV3_should_verify_blob_versioned_hashes_again
return result.Data.Status;
}

public static IEnumerable<TestCaseData> ForkchoiceUpdatedV3DeclinedTestCaseSource
{
get
{
yield return new TestCaseData(Shanghai.Instance, nameof(IEngineRpcModule.engine_forkchoiceUpdatedV3), false)
{
TestName = "ForkchoiceUpdatedV3 To Request Shanghai Payload, Nil Beacon Root",
ExpectedResult = ErrorCodes.InvalidParams,
};
yield return new TestCaseData(Shanghai.Instance, nameof(IEngineRpcModule.engine_forkchoiceUpdatedV3), true)
{
TestName = "ForkchoiceUpdatedV3 To Request Shanghai Payload, Zero Beacon Root",
ExpectedResult = ErrorCodes.UnsupportedFork,
};
yield return new TestCaseData(Shanghai.Instance, nameof(IEngineRpcModule.engine_forkchoiceUpdatedV2), true)
{
TestName = "ForkchoiceUpdatedV2 To Request Shanghai Payload, Zero Beacon Root",
ExpectedResult = ErrorCodes.InvalidParams,
};

yield return new TestCaseData(Cancun.Instance, nameof(IEngineRpcModule.engine_forkchoiceUpdatedV2), true)
{
TestName = "ForkchoiceUpdatedV2 To Request Cancun Payload, Zero Beacon Root",
ExpectedResult = ErrorCodes.InvalidParams,
};
yield return new TestCaseData(Cancun.Instance, nameof(IEngineRpcModule.engine_forkchoiceUpdatedV2), false)
{
TestName = "ForkchoiceUpdatedV2 To Request Cancun Payload, Nil Beacon Root",
ExpectedResult = ErrorCodes.UnsupportedFork,
};
yield return new TestCaseData(Cancun.Instance, nameof(IEngineRpcModule.engine_forkchoiceUpdatedV3), false)
{
TestName = "ForkchoiceUpdatedV3 To Request Cancun Payload, Nil Beacon Root",
ExpectedResult = ErrorCodes.InvalidParams,
};
}
}

public static IEnumerable<TestCaseData> ForkchoiceUpdatedV3AcceptedTestCaseSource
{
get
{
yield return new TestCaseData(Shanghai.Instance, nameof(IEngineRpcModule.engine_forkchoiceUpdatedV2), false)
{
TestName = "ForkchoiceUpdatedV2 To Request Shanghai Payload, Nil Beacon Root",
ExpectedResult = ErrorCodes.None,
};
yield return new TestCaseData(Cancun.Instance, nameof(IEngineRpcModule.engine_forkchoiceUpdatedV3), true)
{
TestName = "ForkchoiceUpdatedV3 To Request Cancun Payload, Zero Beacon Root",
ExpectedResult = ErrorCodes.None,
};
}
}

public static IEnumerable<TestCaseData> BlobVersionedHashesMatchTestSource
{
get
Expand Down
9 changes: 6 additions & 3 deletions src/Nethermind/Nethermind.Merge.Plugin/EngineRpcModule.Paris.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,13 @@ public async Task<ResultWrapper<PayloadStatusV1>> engine_newPayloadV1(ExecutionP

private async Task<ResultWrapper<ForkchoiceUpdatedV1Result>> ForkchoiceUpdated(ForkchoiceStateV1 forkchoiceState, PayloadAttributes? payloadAttributes, int version)
{
if (payloadAttributes?.Validate(_specProvider, version, out string? error) == false)
string? error = null;
switch (payloadAttributes?.Validate(_specProvider, version, out error))
{
if (_logger.IsWarn) _logger.Warn(error);
return ResultWrapper<ForkchoiceUpdatedV1Result>.Fail(error, version >= EngineApiVersions.Cancun ? ErrorCodes.UnsupportedFork : ErrorCodes.InvalidParams);
case PayloadAttributesValidationResult.InvalidParams:
return ResultWrapper<ForkchoiceUpdatedV1Result>.Fail(error!, ErrorCodes.InvalidParams);
case PayloadAttributesValidationResult.UnsupportedFork:
return ResultWrapper<ForkchoiceUpdatedV1Result>.Fail(error!, ErrorCodes.UnsupportedFork);
}

if (await _locker.WaitAsync(_timeout))
Expand Down

0 comments on commit d2dcf5f

Please sign in to comment.