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

Simplify NetworkIdentity type #6666

Merged
merged 13 commits into from
Dec 4, 2024
Merged

Conversation

eddyashton
Copy link
Member

Following #6660, this PR simplifies the NetworkIdentity type by flattening it to a single type. The core of the changes are those in identity.h, which removes ReplicatedNetworkIdentity and several fields from NetworkIdentity. This touches a bunch of other files for 2 reasons:

  • Moved COSESignaturesConfig under the ccf:: namespace, since it is in a public header.
  • Rather than accessing cose_signatures_config from NetworkIdentity, the /join endpoint in node_frontend.h now accesses the single instance stored in/owned by history.h, which takes a few hops to reach. I think some of these hops are helpful, some may be unnecessary, but regardless this fits the existing style.

While we're doing this, be slightly stricter about the order of calls to history, and ensure we throw errors if set_service_signing_identity is called multiple times, or if set_service_signing_identity was not called before either get_cose_signatures_config or emit_signature.

src/node/history.h Outdated Show resolved Hide resolved
@eddyashton eddyashton marked this pull request as ready for review December 4, 2024 09:35
@eddyashton eddyashton requested a review from a team as a code owner December 4, 2024 09:35
src/node/identity.h Outdated Show resolved Hide resolved
@achamayou achamayou enabled auto-merge December 4, 2024 15:31
@achamayou achamayou added this pull request to the merge queue Dec 4, 2024
Merged via the queue into microsoft:main with commit 7ad0ebc Dec 4, 2024
13 checks passed
@achamayou achamayou deleted the identity_refactor branch December 4, 2024 16:30
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