feat: replace amino converters and proto-codecs with verana-frontend sources #268
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
There was a problem hiding this comment. If you're going to generate the proto libraries to be included in the verana-types version, you also need to specify the versions to be used. The issue indicated that the libraries from verana-front that correspond to the currently working version should be copied, which isn't the older version like the ones that were replaced. The idea isn't to generate them again, but to copy those that are known to correspond to the specified protoc version and that work correctly on the front end.
Member
There was a problem hiding this comment. I agree that if we generate these files, the tool and version used should be clear. The real source of truth here is the proto files. The TypeScript codec files are only generated output from those proto definitions, so we should document the generation setup better so local generation, CI and release packaging all use the same recipe. Where I disagree is with not generating them again. If the proto files are the source of truth, then the TypeScript package has to be generated from those proto files, not copied by hand from another repo. This package belongs in And the frontend is not the only consumer here. The same package needs to be usable by the frontend, the indexer, the visualizer, and anything else talking to the chain. What this branch does is bring that back into There was a problem hiding this comment. There are contradictions in what you've written. The goal of the verana-types library is for the amino converters and proto codecs to be packaged and used by the front end and any application that requires them. In other words, there will be no need for local generation; only the published library will be used. Now, the point is that to maintain the library, there are two possibilities:
Member
There was a problem hiding this comment. Thanks, this is clearer. I think this PR is following your option 1, not option 2. In this repo today, the generated codec files are part of the repository, they are regenerated in CI, and they are tested together with the Amino layer before publishing. That is why the codec files are part of this PR. I also agree that the Amino converters may need manual adjustments when proto changes affect signing behavior. That is exactly why I am separating the generated codec files from the handwritten Amino layer. I don’t think option 2 is a good fit here. If the generated codec files only appear at packaging time, then the real published output is no longer fully visible in the PR itself. That makes it harder to review, harder to test end to end, and harder to know later whether a bug came from the proto changes, the generated codec output, the Amino layer, or the packaging step. So for this PR, I think the practical answer is:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,76 @@ | ||
| # Amino Converters | ||
|
|
||
| Amino converters translate between Protobuf messages and Amino JSON format, which is required for Amino (LEGACY_AMINO_JSON) signing with CosmJS. | ||
|
|
||
| ## Structure | ||
|
|
||
| ``` | ||
| amino-converter/ | ||
| cs.ts # Credential Schema module converters | ||
| dd.ts # DID Directory module converters | ||
| perm.ts # Permission module converters | ||
| td.ts # Trust Deposit module converters | ||
| tr.ts # Trust Registry module converters | ||
| util/ | ||
| helpers.ts # Shared conversion helpers | ||
| ``` | ||
|
|
||
| ## Helper Reference | ||
|
|
||
| When creating a new Amino converter, use the helpers from `./util/helpers` based on the field types in the Protobuf message: | ||
|
|
||
| | Proto Field Type | toAmino (Proto -> JSON) | fromAmino (JSON -> Proto) | Notes | | ||
| |---|---|---|---| | ||
| | `uint64` / `Long` | `u64ToStr(value)` | `strToU64(value)` | Amino encodes uint64 as string | | ||
| | `uint32` | `u32ToAmino(value)` | direct assignment | Preserves 0 as `0`, omits `null` | | ||
| | `OptionalUInt32` | `toOptU32Amino(value)` | `fromOptU32Amino(value)` | 0 -> `{}`, n -> `{value: n}`, absent -> `undefined` | | ||
| | `google.protobuf.Timestamp` / `Date` | `dateToIsoAmino(value)` | `isoToDate(value)` | ISO 8601 string, trims `.000Z` to `Z` | | ||
| | `string` | direct assignment | direct assignment | No conversion needed | | ||
| | `bool` | `value ? true : undefined` | `value ?? false` | Omit `false` for omitempty | | ||
| | `string` (optional) | `value ?? ''` | `value ?? ''` | Use empty string default | | ||
|
|
||
| ### Additional helpers | ||
|
|
||
| - **`clean(obj)`**: Removes `undefined` fields from an object. Use when fields should be omitted (omitempty) rather than sent as `null`. | ||
| - **`pickOptionalUInt32(value)`**: Parses loosely-typed input (string, number) into an `OptionalUInt32` wrapper. | ||
|
|
||
| ## Creating a New Amino Converter | ||
|
|
||
| 1. Create a new file in `amino-converter/` named after the module (e.g., `mymodule.ts`). | ||
| 2. Import the Protobuf message types from the codec: `../codec/verana/<module>/v1/tx`. | ||
| 3. Import helpers from `./util/helpers`. | ||
| 4. For each message type, export a converter object with: | ||
| - `aminoType`: the full proto type URL (e.g., `'/verana.mymodule.v1.MsgDoSomething'`) | ||
| - `toAmino(msg)`: converts Proto message to Amino JSON object | ||
| - `fromAmino(value)`: converts Amino JSON back to Proto message using `Msg.fromPartial()` | ||
|
|
||
| ### Field naming convention | ||
|
|
||
| - Proto uses **camelCase** (e.g., `schemaId`, `docUrl`) | ||
| - Amino JSON uses **snake_case** (e.g., `schema_id`, `doc_url`) | ||
|
|
||
| ### Example | ||
|
|
||
| ```typescript | ||
| import { MsgDoSomething } from '../codec/verana/mymodule/v1/tx'; | ||
| import { u64ToStr, strToU64, clean } from './util/helpers'; | ||
|
|
||
| export const MsgDoSomethingAminoConverter = { | ||
| aminoType: '/verana.mymodule.v1.MsgDoSomething', | ||
| toAmino: (msg: MsgDoSomething) => clean({ | ||
| creator: msg.creator ?? '', | ||
| some_id: u64ToStr(msg.someId), // uint64 -> string | ||
| }), | ||
| fromAmino: (value: any) => | ||
| MsgDoSomething.fromPartial({ | ||
| creator: value.creator ?? '', | ||
| someId: strToU64(value.some_id), // string -> uint64 | ||
| }), | ||
| }; | ||
| ``` | ||
|
|
||
| ## Proto Codecs | ||
|
|
||
| Generated with: | ||
| - `protoc-gen-ts_proto` v1.181.2 | ||
| - `protoc` v5.29.3 (libprotoc 29.3) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,87 @@ | ||
| 'use client'; | ||
|
|
||
| import { | ||
| MsgCreateCredentialSchema, | ||
| MsgUpdateCredentialSchema, | ||
| MsgArchiveCredentialSchema, | ||
| } from '../codec/verana/cs/v1/tx'; | ||
| import { u64ToStr, strToU64, u32ToAmino, fromOptU32Amino, toOptU32Amino, clean } from './util/helpers'; | ||
|
|
||
| /** | ||
| * Amino converter for MsgCreateCredentialSchema | ||
| */ | ||
| export const MsgCreateCredentialSchemaAminoConverter = { | ||
| aminoType: '/verana.cs.v1.MsgCreateCredentialSchema', | ||
| // Proto → Amino JSON | ||
| toAmino: (msg: MsgCreateCredentialSchema) => clean({ | ||
| creator: msg.creator ?? '', | ||
| tr_id: u64ToStr(msg.trId), // uint64 -> string | ||
| json_schema: msg.jsonSchema ?? '', | ||
| issuer_grantor_validation_validity_period: toOptU32Amino(msg.issuerGrantorValidationValidityPeriod), | ||
| verifier_grantor_validation_validity_period: toOptU32Amino(msg.verifierGrantorValidationValidityPeriod), | ||
| issuer_validation_validity_period: toOptU32Amino(msg.issuerValidationValidityPeriod), | ||
| verifier_validation_validity_period: toOptU32Amino(msg.verifierValidationValidityPeriod), | ||
| holder_validation_validity_period: toOptU32Amino(msg.holderValidationValidityPeriod), | ||
| issuer_perm_management_mode: u32ToAmino(msg.issuerPermManagementMode) , // uint32 -> number | ||
| verifier_perm_management_mode: u32ToAmino(msg.verifierPermManagementMode) , // uint32 -> number | ||
| }), | ||
| // Amino JSON → Proto | ||
| fromAmino: (value: any ): MsgCreateCredentialSchema => | ||
| MsgCreateCredentialSchema.fromPartial({ | ||
| creator: value.creator ?? '', | ||
| trId: strToU64(value.tr_id), | ||
| jsonSchema: value.json_schema ?? '', | ||
| issuerGrantorValidationValidityPeriod: fromOptU32Amino(value.issuer_grantor_validation_validity_period), | ||
| verifierGrantorValidationValidityPeriod: fromOptU32Amino(value.verifier_grantor_validation_validity_period), | ||
| issuerValidationValidityPeriod: fromOptU32Amino(value.issuer_validation_validity_period), | ||
| verifierValidationValidityPeriod: fromOptU32Amino(value.verifier_validation_validity_period), | ||
| holderValidationValidityPeriod: fromOptU32Amino(value.holder_validation_validity_period), | ||
| issuerPermManagementMode: (value.issuer_perm_management_mode) , | ||
| verifierPermManagementMode: (value.verifier_perm_management_mode) , | ||
| }), | ||
| }; | ||
|
|
||
| /** | ||
| * Amino converter for MsgUpdateCredentialSchema | ||
| */ | ||
| export const MsgUpdateCredentialSchemaAminoConverter = { | ||
| aminoType: '/verana.cs.v1.MsgUpdateCredentialSchema', | ||
| toAmino: (msg: MsgUpdateCredentialSchema) => clean({ | ||
| creator: msg.creator ?? '', | ||
| id: u64ToStr(msg.id), | ||
| issuer_grantor_validation_validity_period: toOptU32Amino(msg.issuerGrantorValidationValidityPeriod), | ||
| verifier_grantor_validation_validity_period: toOptU32Amino(msg.verifierGrantorValidationValidityPeriod), | ||
| issuer_validation_validity_period: toOptU32Amino(msg.issuerValidationValidityPeriod), | ||
| verifier_validation_validity_period: toOptU32Amino(msg.verifierValidationValidityPeriod), | ||
| holder_validation_validity_period: toOptU32Amino(msg.holderValidationValidityPeriod), | ||
| }), | ||
| fromAmino: (value: any) => MsgUpdateCredentialSchema.fromPartial({ | ||
| creator: value.creator ?? '', | ||
| id: strToU64(value.id), | ||
| issuerGrantorValidationValidityPeriod: fromOptU32Amino(value.issuer_grantor_validation_validity_period), | ||
| verifierGrantorValidationValidityPeriod: fromOptU32Amino(value.verifier_grantor_validation_validity_period), | ||
| issuerValidationValidityPeriod: fromOptU32Amino(value.issuer_validation_validity_period), | ||
| verifierValidationValidityPeriod: fromOptU32Amino(value.verifier_validation_validity_period), | ||
| holderValidationValidityPeriod: fromOptU32Amino(value.holder_validation_validity_period), | ||
| }), | ||
| }; | ||
|
|
||
| /** | ||
| * Amino converter for MsgArchiveCredentialSchema | ||
| */ | ||
| export const MsgArchiveCredentialSchemaAminoConverter = { | ||
| aminoType: '/verana.cs.v1.MsgArchiveCredentialSchema', | ||
| // Proto → Amino JSON | ||
| toAmino: (msg: MsgArchiveCredentialSchema) => clean({ | ||
| creator: msg.creator ?? '', | ||
| id: u64ToStr(msg.id), | ||
| archive: msg.archive ? true : undefined, // omit if false | ||
| }), | ||
| // Amino JSON → Proto | ||
| fromAmino: (value: any): MsgArchiveCredentialSchema => | ||
| MsgArchiveCredentialSchema.fromPartial({ | ||
| creator: value.creator, | ||
| id: strToU64(value.id), | ||
| archive: value.archive ?? false, | ||
| }), | ||
| }; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,72 @@ | ||
| 'use client'; | ||
|
|
||
| import { AminoConverter } from '@cosmjs/stargate' | ||
| import { MsgAddDID, MsgRenewDID, MsgTouchDID, MsgRemoveDID } from '../codec/verana/dd/v1/tx' | ||
|
|
||
| /** | ||
| * Amino converter for MsgAddDID | ||
| */ | ||
| export const MsgAddDIDAminoConverter: AminoConverter = { | ||
| aminoType: '/verana.dd.v1.MsgAddDID', | ||
| toAmino: (msg: MsgAddDID) => ({ | ||
| creator: msg.creator, | ||
| did: msg.did, | ||
| years: msg.years, | ||
| }), | ||
| fromAmino: (value: any) => | ||
| MsgAddDID.fromPartial({ | ||
| creator: value.creator, | ||
| did: value.did, | ||
| years: value.years, | ||
| }), | ||
| } | ||
|
|
||
| /** | ||
| * Amino converter for MsgRenewDID | ||
| */ | ||
| export const MsgRenewDIDAminoConverter: AminoConverter = { | ||
| aminoType: '/verana.dd.v1.MsgRenewDID', | ||
| toAmino: (msg: MsgRenewDID) => ({ | ||
| creator: msg.creator, | ||
| did: msg.did, | ||
| years: msg.years, | ||
| }), | ||
| fromAmino: (value: any) => | ||
| MsgRenewDID.fromPartial({ | ||
| creator: value.creator, | ||
| did: value.did, | ||
| years: value.years, | ||
| }), | ||
| } | ||
|
|
||
| /** | ||
| * Amino converter for MsgTouchDID | ||
| */ | ||
| export const MsgTouchDIDAminoConverter: AminoConverter = { | ||
| aminoType: '/verana.dd.v1.MsgTouchDID', | ||
| toAmino: (msg: MsgTouchDID) => ({ | ||
| creator: msg.creator, | ||
| did: msg.did, | ||
| }), | ||
| fromAmino: (value: any) => | ||
| MsgTouchDID.fromPartial({ | ||
| creator: value.creator, | ||
| did: value.did, | ||
| }), | ||
| } | ||
|
|
||
| /** | ||
| * Amino converter for MsgRemoveDID | ||
| */ | ||
| export const MsgRemoveDIDAminoConverter: AminoConverter = { | ||
| aminoType: '/verana.dd.v1.MsgRemoveDID', | ||
| toAmino: (msg: MsgRemoveDID) => ({ | ||
| creator: msg.creator, | ||
| did: msg.did, | ||
| }), | ||
| fromAmino: (value: any) => | ||
| MsgRemoveDID.fromPartial({ | ||
| creator: value.creator, | ||
| did: value.did, | ||
| }), | ||
| } |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Apparently, github.event.number holds the number assigned to the PR it is running from. So the resulting tag would be
pr-268in this case. This would be IMHO a good solution, since the app consuming this PR will be usually interested in getting the last version released for it.I didn't test it by myself but that's the approach we follow in Credo and shown in some examples on GitHub documentation.
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.
@genaris Oh okay that's cool thanks, I didn't even know this was a thing. It's better to use this since it's unique but also predictable for us and doesn't change