-
Notifications
You must be signed in to change notification settings - Fork 286
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
feat(consortium-static): new consortium plugin #3471
feat(consortium-static): new consortium plugin #3471
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@eduv09 Please include all details in the commit message not just the PR description
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@eduv09 Looks like a high quality PR, thank you very much! I have some nit-picks in the comments above and a couple requests here:
- Remove the changelog file, it doesn't make sense to have it in a new package - it gets auto-generated when we issue a release.
- Add a CI job in ci.yaml that runs the Jest tests so that it can be verified to keep working after each PR. Note that the github action versions to use are changing so make sure to use the up to date versions as shown in this otherwise unrelated PR: https://github.com/hyperledger/cacti/pull/3478
...cti-plugin-consortium-static/src/main/typescript/consortium/process-broadcast-endpoint-v1.ts
Outdated
Show resolved
Hide resolved
...cti-plugin-consortium-static/src/main/typescript/consortium/process-broadcast-endpoint-v1.ts
Outdated
Show resolved
Hide resolved
...ges/cacti-plugin-consortium-static/src/main/typescript/repository/new-consortium-provider.ts
Outdated
Show resolved
Hide resolved
...s/cacti-plugin-consortium-static/src/main/typescript/repository/new-consortium-repository.ts
Outdated
Show resolved
Hide resolved
packages/cacti-plugin-consortium-static/src/main/typescript/plugin-consortium-static.ts
Outdated
Show resolved
Hide resolved
@eduv09 One more thing: if you run |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is very high-quality work, thanks for your contribution! I've left a few comments, most things should be relatively easy to fix. Please also refer to @petermz 's comments. Besides that, LGTM!
packages/cacti-plugin-consortium-static/src/main/json/openapi.json
Outdated
Show resolved
Hide resolved
packages/cacti-plugin-consortium-static/src/main/typescript/utils.ts
Outdated
Show resolved
Hide resolved
packages/cacti-plugin-consortium-static/src/main/typescript/plugin-consortium-static.ts
Show resolved
Hide resolved
packages/cacti-plugin-consortium-static/src/main/typescript/plugin-consortium-static.ts
Outdated
Show resolved
Hide resolved
packages/cacti-plugin-consortium-static/src/main/typescript/policy-model/polity-item.ts
Show resolved
Hide resolved
c75f8b2
to
1eb0da1
Compare
@RafaelAPB @petermetz I think I addressed all of the changes you requested. Let me know if I missed something, or of any other recommendations. Thank you ! |
@eduv09 please rebase and fix any conflicts or errors |
cb313ff
to
0388f09
Compare
@RafaelAPB Done |
LGTM, let's wait for @petermetz 's re-review. Cheers |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@eduv09 @RafaelAPB LGTM, thank you for addressing my nit-picks!
For the JWSGeneral
casting question, please see my analysis here (short answer, let's merge this as is and I'll work on fixing the casting) => https://github.com/hyperledger/cacti/pull/3471#discussion_r1731894747
One more thing: If you hit the re-request review button it will automatically land back in my PR review queue (which I try to check daily even when I don't have time to process discord messages) so I recommend hitting that every time a PR I reviewed is ready for some more attention. Tagging me on Discord and on GitHub also helps so I recommend you also keep doing that of course!
1. createAjvTypeGuard<T>() is the lower level utility which can be used to construct the more convenient, higher level type predicates/type guards such as createIsJwsGeneralTypeGuard() which uses createAjvTypeGuard<JwsGeneral> under the hood. 2. This commit is also meant to be establishing a larger, more generic pattern of us being able to create type guards out of the Open API specs in a convenient way instead of having to write the validation code by hand. An example usage of the new createAjvTypeGuard<T>() utility is the createIsJwsGeneralTypeGuard() function itself. An example usage of the new createIsJwsGeneralTypeGuard() can be found in packages/cactus-plugin-consortium-manual/src/main/typescript/plugin-consortium-manual.ts The code documentation contains examples as well for maximum discoverabilty and I'll also include it here: ```typescript import { JWSGeneral } from "@hyperledger/cactus-core-api"; import { createIsJwsGeneralTypeGuard } from "@hyperledger/cactus-core-api"; export class PluginConsortiumManual { private readonly isJwsGeneral: (x: unknown) => x is JWSGeneral; constructor() { // Creating the type-guard function is relatively costly due to the Ajv schema // compilation that needs to happen as part of it so it is good practice to // cache the type-guard function as much as possible, for examle by adding it // as a class member on a long-lived object such as a plugin instance which is // expected to match the life-cycle of the API server NodeJS process itself. // The specific anti-pattern would be to create a new type-guard function // for each request received by a plugin as this would affect performance // negatively. this.isJwsGeneral = createIsJwsGeneralTypeGuard(); } public async getNodeJws(): Promise<JWSGeneral> { // rest of the implementation that produces a JWS ... const jws = await joseGeneralSign.sign(); if (!this.isJwsGeneral(jws)) { throw new TypeError("Jose GeneralSign.sign() gave non-JWSGeneral type"); } return jws; } } ``` Relevant discussion took place here: https://github.com/hyperledger/cacti/pull/3471#discussion_r1731894747 Signed-off-by: Peter Somogyvari <[email protected]>
1. createAjvTypeGuard<T>() is the lower level utility which can be used to construct the more convenient, higher level type predicates/type guards such as createIsJwsGeneralTypeGuard() which uses createAjvTypeGuard<JwsGeneral> under the hood. 2. This commit is also meant to be establishing a larger, more generic pattern of us being able to create type guards out of the Open API specs in a convenient way instead of having to write the validation code by hand. An example usage of the new createAjvTypeGuard<T>() utility is the createIsJwsGeneralTypeGuard() function itself. An example usage of the new createIsJwsGeneralTypeGuard() can be found in packages/cactus-plugin-consortium-manual/src/main/typescript/plugin-consortium-manual.ts The code documentation contains examples as well for maximum discoverabilty and I'll also include it here: ```typescript import { JWSGeneral } from "@hyperledger/cactus-core-api"; import { createIsJwsGeneralTypeGuard } from "@hyperledger/cactus-core-api"; export class PluginConsortiumManual { private readonly isJwsGeneral: (x: unknown) => x is JWSGeneral; constructor() { // Creating the type-guard function is relatively costly due to the Ajv schema // compilation that needs to happen as part of it so it is good practice to // cache the type-guard function as much as possible, for examle by adding it // as a class member on a long-lived object such as a plugin instance which is // expected to match the life-cycle of the API server NodeJS process itself. // The specific anti-pattern would be to create a new type-guard function // for each request received by a plugin as this would affect performance // negatively. this.isJwsGeneral = createIsJwsGeneralTypeGuard(); } public async getNodeJws(): Promise<JWSGeneral> { // rest of the implementation that produces a JWS ... const jws = await joseGeneralSign.sign(); if (!this.isJwsGeneral(jws)) { throw new TypeError("Jose GeneralSign.sign() gave non-JWSGeneral type"); } return jws; } } ``` Relevant discussion took place here: https://github.com/hyperledger/cacti/pull/3471#discussion_r1731894747 Signed-off-by: Peter Somogyvari <[email protected]>
0388f09
to
55c186b
Compare
* New plugin consortium-static, based on consortium-manual. Long story short, the plugin allows for the addition of Cacti Nodes to consortium at runtime. New node entities must belong to one of the consortium entities (organizations) that were specified on consortium creation. So it allows to add new nodes 'CactusNode', but not new 'ConsortiumMember' entities. * New feature: add CactusNode to consortium at runtime The code is based on the plugin-consortium-manual, with the new features built on top of it. The process of adding a new node is conducted as follows: 1. The new node forges a request with information about itself and sends it to any Node in the consortium 2. The receiver verifies the request, and if it is approved, broadcasts it to the rest of the consortium Nodes. 3. Each node verifies the same request, and adds the new node to the consortium database. At the same time, the receiver node from point 2 sends the consortium data to the new node, who updates it's database. * Includes a new policy model The package includes a policy model, which was developed based on the ideas of the Policy Core Information Model [RFC3060](https://www.rfc-editor.org/rfc/rfc3060) . The idea was to introduce the idea of a multi-purpose policy framework that can be leveraged by other packages in cacti. Signed-off-by: eduv09 <[email protected]>
55c186b
to
977c816
Compare
@petermetz fixed the CI workflow issue. I think it's good to go. Thank you for the assist 🙏 |
@eduv09 You got it! Thank you very much for the contribution! |
1. createAjvTypeGuard<T>() is the lower level utility which can be used to construct the more convenient, higher level type predicates/type guards such as createIsJwsGeneralTypeGuard() which uses createAjvTypeGuard<JwsGeneral> under the hood. 2. This commit is also meant to be establishing a larger, more generic pattern of us being able to create type guards out of the Open API specs in a convenient way instead of having to write the validation code by hand. An example usage of the new createAjvTypeGuard<T>() utility is the createIsJwsGeneralTypeGuard() function itself. An example usage of the new createIsJwsGeneralTypeGuard() can be found in packages/cactus-plugin-consortium-manual/src/main/typescript/plugin-consortium-manual.ts The code documentation contains examples as well for maximum discoverabilty and I'll also include it here: ```typescript import { JWSGeneral } from "@hyperledger/cactus-core-api"; import { createIsJwsGeneralTypeGuard } from "@hyperledger/cactus-core-api"; export class PluginConsortiumManual { private readonly isJwsGeneral: (x: unknown) => x is JWSGeneral; constructor() { // Creating the type-guard function is relatively costly due to the Ajv schema // compilation that needs to happen as part of it so it is good practice to // cache the type-guard function as much as possible, for examle by adding it // as a class member on a long-lived object such as a plugin instance which is // expected to match the life-cycle of the API server NodeJS process itself. // The specific anti-pattern would be to create a new type-guard function // for each request received by a plugin as this would affect performance // negatively. this.isJwsGeneral = createIsJwsGeneralTypeGuard(); } public async getNodeJws(): Promise<JWSGeneral> { // rest of the implementation that produces a JWS ... const jws = await joseGeneralSign.sign(); if (!this.isJwsGeneral(jws)) { throw new TypeError("Jose GeneralSign.sign() gave non-JWSGeneral type"); } return jws; } } ``` Relevant discussion took place here: https://github.com/hyperledger/cacti/pull/3471#discussion_r1731894747 Signed-off-by: Peter Somogyvari <[email protected]>
1. createAjvTypeGuard<T>() is the lower level utility which can be used to construct the more convenient, higher level type predicates/type guards such as createIsJwsGeneralTypeGuard() which uses createAjvTypeGuard<JwsGeneral> under the hood. 2. This commit is also meant to be establishing a larger, more generic pattern of us being able to create type guards out of the Open API specs in a convenient way instead of having to write the validation code by hand. An example usage of the new createAjvTypeGuard<T>() utility is the createIsJwsGeneralTypeGuard() function itself. An example usage of the new createIsJwsGeneralTypeGuard() can be found in packages/cactus-plugin-consortium-manual/src/main/typescript/plugin-consortium-manual.ts The code documentation contains examples as well for maximum discoverabilty and I'll also include it here: ```typescript import { JWSGeneral } from "@hyperledger/cactus-core-api"; import { createIsJwsGeneralTypeGuard } from "@hyperledger/cactus-core-api"; export class PluginConsortiumManual { private readonly isJwsGeneral: (x: unknown) => x is JWSGeneral; constructor() { // Creating the type-guard function is relatively costly due to the Ajv schema // compilation that needs to happen as part of it so it is good practice to // cache the type-guard function as much as possible, for examle by adding it // as a class member on a long-lived object such as a plugin instance which is // expected to match the life-cycle of the API server NodeJS process itself. // The specific anti-pattern would be to create a new type-guard function // for each request received by a plugin as this would affect performance // negatively. this.isJwsGeneral = createIsJwsGeneralTypeGuard(); } public async getNodeJws(): Promise<JWSGeneral> { // rest of the implementation that produces a JWS ... const jws = await joseGeneralSign.sign(); if (!this.isJwsGeneral(jws)) { throw new TypeError("Jose GeneralSign.sign() gave non-JWSGeneral type"); } return jws; } } ``` Relevant discussion took place here: https://github.com/hyperledger/cacti/pull/3471#discussion_r1731894747 Signed-off-by: Peter Somogyvari <[email protected]>
New plugin consortium-static, based on consortium-manual.
Long story short, the plugin allows for the addition of Cacti Nodes to consortium at runtime. New node entities must belong to one of the consortium entities (organizations) that were specified on consortium creation. So it allows to add new nodes 'CactusNode', but not new 'ConsortiumMember' entities.
New feature: add CactusNode to consortium at runtime
The code is based on the plugin-consortium-manual, with the new features built on top of it. The process of adding a new node is conducted as follows:
Includes a new policy model
The package includes a policy model, which was developed based on the ideas of the Policy Core Information Model RFC3060 . The idea was to introduce the idea of a multi-purpose policy framework that can be leveraged by other packages in cacti.