-
Notifications
You must be signed in to change notification settings - Fork 3.5k
Fix private-data TypeScript chaincode #1357
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
Fix private-data TypeScript chaincode #1357
Conversation
The build is only testing the Go chaincode for the asset-transfer-private-data sample. The behavior of the TypeScript chaincode implementation is not consistent with the Go and Java versions, which prevents it from working correctly. This change fixes the TypeScript chaincode implementation for the asset-transfer-private-data sample so that it works correctly. Additionally, some JSON property names are explicitly set in the Go client application sample, which prevented it from working with the Java and TypeScript chaincode implementations. Signed-off-by: Mark S. Lewis <[email protected]>
Tsafaras
left a comment
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.
I tested it and working as expected! Thanks for the quick fix.
Left some questions that I have.
| } | ||
|
|
||
| export class TransientAssetOwner { | ||
| export interface TransientAssetOwner { |
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.
How come you turned this and TransientAssetValue into interfaces?
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.
The sort-keys-recursive package fails on class instances. It only works on plain objects or arrays.
| throw new Error('no asset data'); | ||
| } | ||
| const json = Buffer.from(bytes).toString(); | ||
| const json = Buffer.from(bytes).toString('utf8'); |
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.
Is there a reason for explicitly stating the encoding in this toString?
I'm asking because you didn't add it to the rest of the calls that come up in this commit's diff.
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.
I guess I changed this when I was trying to figure out what was causing a breakage here. utf8 is the default for the Buffer toString() method so it really should not matter whether it is stated explicitly or not. In the past I have seen protobuf object implementations that mimic Buffer behave differently and fail if the encoding is not explicitly specified. That isn't the case here though.
|
|
||
| constructor(transientMap: Map<string, Uint8Array>) { | ||
| const transient = transientMap.get("asset_properties"); | ||
| const transient = transientMap.get('asset_properties'); |
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.
The project could use a strict formatter, no? Like biome.
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.
There is a top-level .editorconfig file that currently specifies single-quotes in ts/js files. It isn't strictly enforced across the repo but my IDE is using it. I also use Prettier for opinionated ts/js code formatting. I guess Biome should give almost identical results. Again, this is not enforced in this repo.
The build is only testing the Go chaincode for the asset-transfer-private-data sample. The behavior of the TypeScript chaincode implementation is not consistent with the Go and Java versions, which prevents it from working correctly.
This change fixes the TypeScript chaincode implementation for the asset-transfer-private-data sample so that it works correctly. Additionally, some JSON property names are explicitly set in the Go client application sample, which prevented it from working with the Java and TypeScript chaincode implementations.