Skip to content

Conversation

shrimalmadhur
Copy link
Contributor

Why are these changes needed?

Checks

  • I've made sure the tests are passing. Note that there might be a few flaky tests, in that case, please comment that they are not relevant.
  • I've checked the new test coverage and the coverage percentage didn't drop.
  • Testing Strategy
    • Unit tests
    • Integration tests
    • This PR is not tested :(

Comment on lines +190 to +192
if blsRemoteSignerEnabled && blsSignerAPIKey == "" {
return nil, fmt.Errorf("BLS signer API key is required if BLS remote signer is enabled")
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this requirement predicated on a new version of Cerberus that also requires API key? I've got remote BLS running on my testnet now with Cerberus. When I upgrade my node, do I need to simultaneously upgrade Cerberus? If I upgrade Cerberus first, will my node fail to remote sign until upgraded with this change?

Copy link
Contributor Author

@shrimalmadhur shrimalmadhur Jan 18, 2025

Choose a reason for hiding this comment

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

yes basically you will have to clean import on the new version of cerberus - since it sets up a DB and then persists some metadata for the keys - if you are already running cerberus. You will have to upgrade both together
not ideal but given only few folks (2-3 folks outside us) are testing right now on testnet it's an okay process. wdyt?

Basically steps

  • Upgrade Cerberus (with postgres)
  • Reimport key which will give you API key
  • Add that in .env on eigenda with latest eigenda release
  • restart eigenda

there could be missed batches in between which won't be able to sign due to this inconsistency

Copy link
Contributor

Choose a reason for hiding this comment

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

Yea, as long as current testnet users are in the loop, seems ok.

If possible might make sense to disable remote singing on node as first step - reverting to local BLS, to allow for Cerberus upgrade validation outside production path. Or maybe recommend users spin up new Cerberus alongside old Cerberus on new port so that they can import and validate new Cerberus is ready before pulling the switch. Then update .env and bump node release.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

that's a good point. I will write an upgrade guide in release notes with these options and share. good call on that.

@shrimalmadhur shrimalmadhur marked this pull request as ready for review January 18, 2025 00:28
@shrimalmadhur shrimalmadhur merged commit 7850088 into master Jan 18, 2025
9 checks passed
@samlaf samlaf deleted the madhur/add-api-key-cerberus branch January 18, 2025 16:54
0x0aa0 pushed a commit that referenced this pull request Jan 21, 2025
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