Skip to content

Replace signingRoot property with signing_root - #6

Open
usmansaleem wants to merge 1 commit into
ethereum:mainfrom
usmansaleem:signing_root
Open

usmansaleem wants to merge 1 commit into
ethereum:mainfrom
usmansaleem:signing_root

Conversation

@usmansaleem

Copy link
Copy Markdown
Collaborator

Replace signingRoot property with signing_root as all other properties are snake case.

Fixes #5

@james-prysm

Copy link
Copy Markdown
Collaborator

I think this change will need some reflection on the migration path. should we be introducing a new version or something?

@usmansaleem

Copy link
Copy Markdown
Collaborator Author

I think this change will need some reflection on the migration path. should we be introducing a new version or something?

The spec that is on the main branch can be treated as dev version. The spec that is available on the github release page (and is tagged) v1.0.0 is the one that web3signer currently implemented and other validator clients also support. The Deneb changes should be part of version release v1.1.0, for example. Similarly, signing_root change should be part of either 1.2.0 or 2.0.0.

@jframe

jframe commented Aug 22, 2023

Copy link
Copy Markdown
Collaborator

If this is merged in though this would be part of the next release of the API v1.1.0 along with the Deneb changes and it would be a breaking change.

Would prefer that we had a release of the API with the Deneb without any breaking changes to the signing root and put this in the next release 1.2.0.

I think this change will need some reflection on the migration path. should we be introducing a new version or something?

The spec that is on the main branch can be treated as dev version. The spec that is available on the github release page (and is tagged) v1.0.0 is the one that web3signer currently implemented and other validator clients also support. The Deneb changes should be part of version release v1.1.0, for example. Similarly, signing_root change should be part of either 1.2.0 or 2.0.0.

@usmansaleem

Copy link
Copy Markdown
Collaborator Author

If this is merged in though this would be part of the next release of the API v1.1.0 along with the Deneb changes and it would be a breaking change.

Would prefer that we had a release of the API with the Deneb without any breaking changes to the signing root and put this in the next release 1.2.0.

Agreed, this PR should be merged after releasing 1.1.0 and/or, create/push tag with deneb related commit.

@usmansaleem usmansaleem added this to the v1.2.0 milestone Aug 22, 2023
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.

Change signingRoot to signing_root

3 participants