GetBodiesByRangeV1 implementation #4939
Conversation
deffrian
changed the title
src/Nethermind/Nethermind.Merge.Plugin/Handlers/V1/GetPayloadBodiesByRangeV1Handler.cs
Show resolved
Hide resolved
| { | ||
| List<ExecutionPayloadBodyV1Result> payloadBodies = new(blockHashes.Length); | ||
| List<ExecutionPayloadBodyV1Result?> payloadBodies = new(blockHashes.Length); |
There was a problem hiding this comment.
use Array here not list, can avoid later ToArray call.
src/Nethermind/Nethermind.Merge.Plugin/Handlers/V1/GetPayloadBodiesByRangeV1Handler.cs
Show resolved
Hide resolved
src/Nethermind/Nethermind.Merge.Plugin/Handlers/V1/GetPayloadBodiesByRangeV1Handler.cs
Show resolved
Hide resolved
| ErrorCodes.LimitExceeded); | ||
| } | ||
|
|
||
| List<ExecutionPayloadBodyV1Result?> payloadBodies = new((int)count); |
| var payloadBodies = new ExecutionPayloadBodyV1Result?[count]; | ||
| for (int i = 0; i < count; i++) | ||
| { | ||
| Block? block = _blockTree.FindBlock(start + i); | ||
| payloadBodies[i] = block is not null ? new ExecutionPayloadBodyV1Result(block.Transactions) : null; | ||
| } |
There was a problem hiding this comment.
Question - maybe even better would be to change result from array to IEnumerable<> and go with yield return? Would avoid even more allocations.
| var payloadBodies = new ExecutionPayloadBodyV1Result?[blockHashes.Length]; | ||
| for (int i = 0; i < blockHashes.Length; ++i) | ||
| { | ||
| Block? block = _blockTree.FindBlock(hash); | ||
| if (block is not null) | ||
| { | ||
| payloadBodies.Add(new ExecutionPayloadBodyV1Result(block.Transactions)); | ||
| } | ||
| Block? block = _blockTree.FindBlock(blockHashes[i]); | ||
|
|
||
| payloadBodies[i] = block is not null ? new ExecutionPayloadBodyV1Result(block.Transactions) : null; | ||
| } | ||
|
|
||
| return Task.FromResult(ResultWrapper<ExecutionPayloadBodyV1Result[]>.Success(payloadBodies.ToArray())); | ||
| return Task.FromResult(ResultWrapper<ExecutionPayloadBodyV1Result?[]>.Success(payloadBodies)); |
There was a problem hiding this comment.
Question - maybe even better would be to change result from array to IEnumerable<> and go with yield return? Would avoid even more allocations.
There was a problem hiding this comment.
Would play less nicely with Exception though, as this would be evaluated during serialization.
There was a problem hiding this comment.
One other idea to explore. With this branch: feature/json_dispose_result , we could use ArrayPoolList here. Just need to also dispose it if exception is thrown.
Fixes | Closes | Resolves #
Changes:
getPayloadBodiesByRangeV1to #146 ethereum/execution-apis#218Types of changes
What types of changes does your code introduce?
Put an
xin the boxes that applyTesting
Have been tested manually with sepolia node.
Hive tests:
rpc
rpc-compat
engine
Requires testing
In case you checked yes, did you write tests??