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

Commit Boost Acceptance Test for List Pub Keys #1040

Merged
merged 10 commits into from
Nov 26, 2024

Conversation

usmansaleem
Copy link
Contributor

PR Description

Add commit boost acceptance test for list pub keys API. Also fix the public key identifiers used by commit boost by introducing K256ArtifactSigner.

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.

Testing

  • I thought about testing these changes in a realistic/non-local environment.

@usmansaleem usmansaleem self-assigned this Nov 22, 2024
@usmansaleem usmansaleem added the TeamCerberus Under active development by TeamCerberus @Consensys label Nov 22, 2024
* @param identifier The identifier for which to sign data.
* @param data Bytes which is signed
* @return Optional ArtifactSignature of type T. Empty if no signer available for given identifier
*/
@SuppressWarnings("unchecked")
public <T extends ArtifactSignature<?>> Optional<T> signAndGetArtifactSignature(
Copy link
Contributor

Choose a reason for hiding this comment

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

Removing this means we need to cast in some places

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is currently used only at one place, in TransactionSerializer.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Let me try to see if we can eliminate this method altogether.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jframe I've eliminated this method. TransactionSerializer can reconstruct SignatureData from the hex String.

 -- derive SignatureData from existing SecpArtifactSignature builder method
@usmansaleem usmansaleem merged commit f27d7ea into Consensys:master Nov 26, 2024
6 checks passed
@usmansaleem usmansaleem deleted the cb_at_dsl branch November 26, 2024 08:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
TeamCerberus Under active development by TeamCerberus @Consensys
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants