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

removed requirement for block headers in voluntary exit #8461

Merged
merged 6 commits into from
Jul 22, 2024

Conversation

rolfyone
Copy link
Contributor

Instead of using the block header we can compute the current slot from genesis data.

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.

rolfyone added 2 commits July 22, 2024 14:35
Instead of using the block header we can compute the current slot from genesis data.

Signed-off-by: Paul Harris <[email protected]>
Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

Removed the requirement for block headers in voluntary exit operations, now computing the current slot from genesis data.

  • Modified teku/src/integration-test/java/tech/pegasys/teku/cli/subcommand/VoluntaryExitCommandTest.java to use genesis data for slot computation.
  • Deleted teku/src/integration-test/resources/tech/pegasys/teku/cli/subcommand/voluntary-exit/genesis.json as genesis data is no longer required.
  • Updated teku/src/main/java/tech/pegasys/teku/cli/subcommand/VoluntaryExitCommand.java to compute the current epoch from genesis data.
  • Removed getBlockHeader method from validator/remote/src/main/java/tech/pegasys/teku/validator/remote/apiclient/OkHttpValidatorRestApiClient.java.

4 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings

…ochs and save to file, but thats a valid scenario so put it back and put in a test case for it.

Signed-off-by: Paul Harris <[email protected]>
Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

(updates since last review)

Removed the requirement for block headers in voluntary exit operations, now computing the current slot from genesis data.

  • Added shouldCreateExitForFutureEpochIfOutputFolderDefined test in teku/src/integration-test/java/tech/pegasys/teku/cli/subcommand/VoluntaryExitCommandTest.java to verify future epoch exits.
  • Modified teku/src/main/java/tech/pegasys/teku/cli/subcommand/VoluntaryExitCommand.java to allow future epoch exits if voluntaryExitsFolder is specified.
  • Updated validation logic in VoluntaryExitCommand.java to enhance flexibility while handling exit messages.

2 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

          ## PR Summary

          (updates since last review)


          Removed the requirement for block headers in voluntary exit operations, now computing the current slot from genesis data.
  • Updated acceptance-tests/src/acceptance-test/java/tech/pegasys/teku/test/acceptance/MergedGenesisAcceptanceTest.java to use genesisExecutionPayloadHeaderSource for genesis payload.

  • Modified acceptance-tests/src/testFixtures/java/tech/pegasys/teku/test/acceptance/dsl/GenesisGenerator.java to replace genesisPayloadSource with genesisExecutionPayloadHeaderSource.

  • Enhanced modularity and configurability in genesis generation by using a function that takes a Spec and returns an ExecutionPayloadHeader.

        <sub>2 file(s) reviewed, no comment(s)</sub>
        <sub>[Edit PR Review Bot Settings](https://app.greptile.com/apps/github)</sub>
    

Copy link
Contributor

@tbenr tbenr left a comment

Choose a reason for hiding this comment

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

LGTM but InvalidConfigurationException message needs to be updated

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

(updates since last review)

Removed the requirement for block headers in voluntary exit operations, now computing the current slot from genesis data.

  • Modified teku/src/main/java/tech/pegasys/teku/cli/subcommand/VoluntaryExitCommand.java to update error messages reflecting the shift from block headers to genesis data for epoch calculation.
  • Adjusted teku/src/integration-test/java/tech/pegasys/teku/cli/subcommand/VoluntaryExitCommandTest.java to ensure genesis response configuration before executing voluntary exit commands.
  • Added new test methods in teku/src/integration-test/java/tech/pegasys/teku/cli/subcommand/VoluntaryExitCommandTest.java to handle scenarios where genesis data is unavailable or the beacon node is not accessible.

2 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

(updates since last review)

Removed the requirement for block headers in voluntary exit operations, now computing the current slot from genesis data.

  • Updated data/provider/src/main/java/tech/pegasys/teku/api/DataProvider.java to include recentChainData and spec in NodeDataProvider constructor.
  • Enhanced data/provider/src/main/java/tech/pegasys/teku/api/NodeDataProvider.java with methods for handling attestations and metadata, and computing the current slot.
  • Improved test coverage in data/provider/src/test/java/tech/pegasys/teku/api/NodeDataProviderTest.java by adding new test cases for getAttestationsAndMetaData and slot retrieval scenarios.

3 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings

@rolfyone rolfyone merged commit 2d2d17b into Consensys:master Jul 22, 2024
16 checks passed
@rolfyone rolfyone deleted the exit-move-fn2 branch July 22, 2024 21:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants