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

Starting from transitioned initial anchor with tests #8221

Draft
wants to merge 14 commits into
base: master
Choose a base branch
from

Conversation

zilm13
Copy link
Contributor

@zilm13 zilm13 commented Apr 18, 2024

PR Description

Based on #7981
Work in progress.

Fixed Issue(s)

Documentation

  • I thought about documentation and added the doc-change-required label to this PR if updates are required.

Changelog

  • I thought about adding a changelog entry, and added one if I deemed necessary.

@zilm13 zilm13 added the DO NOT MERGE Not ready to merge label Apr 18, 2024
import tech.pegasys.teku.spec.schemas.SchemaDefinitionsBellatrix;
import tech.pegasys.teku.spec.util.DataStructureUtil;

class MiscHelpersBellatrixTest {

Check notice

Code scanning / CodeQL

Unused classes and interfaces Note test

Unused class: MiscHelpersBellatrixTest is not referenced within this codebase. If not used as an external API it should be removed.
@@ -69,6 +69,7 @@ public void handleRequest(RestApiRequest request) throws JsonProcessingException
request.header(Header.CACHE_CONTROL, CACHE_NONE);

final String blockId = request.getPathParameter(PARAMETER_BLOCK_ID);
// FIXME: not sure. Are we ok to return here transitioned state on keyword finalized?
Copy link
Contributor

Choose a reason for hiding this comment

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

my understand is that this is ok since this endpoint should mimic the same behaviour as the /eth/v2/debug/beacon/states/{state_id} but allowing to search for block. We're using the same blockroot for the transitioned state so it looks logical to me that we returned the transitioned state

@@ -375,6 +376,7 @@ private Optional<ReorgContext> computeReorgContext(
return optionalReorgContext;
}

// TODO: Test the consequences of having transitioned chainHead
Copy link
Contributor

Choose a reason for hiding this comment

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

why would we transition the chainHead? I thought we were only going to transition finalized states/block

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DO NOT MERGE Not ready to merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants