Implement TypeScript version of asset-transfer-ledger-queries #1309
Conversation
This commit implements the TypeScript version of the asset-transfer-ledger-queries Chaincode. Resolves hyperledger#1232 Signed-off-by: Satoshi Ito <[email protected]>
There was a problem hiding this comment.
This looks great. Some small changes I think are needed before merging:
- Remove
tslint.json. The sample uses eslint (with typescript-eslint), not the deprecated tslint. I don't think think this tslint configuration file is used. - Include an
npm-shrinkwrap.jsonfile to minimise the chances of future breakages when dependencies release new versions by pinning the resolved dependency versions. Just runnpm install && npm shrinkwraplocally, then check in the created shrinkwrap file. - Add a
.gitignorein theasset-transfer-ledger-queries/chaincode-typescriptdirectory to ignore thedistsubdirectory generated by the TypeScript compilation. - Add
typescriptto the chaincode-language matrix in.github/workflows/test-network-ledger.yamlso this chaincode is tested by the automated build.
There was a problem hiding this comment.
@satoshi-ito-ht
Thank you for your contribution.
I've also added some review comments, so please check them out.
|
|
||
| @Info({ | ||
| title: 'AssetTransfer', | ||
| description: 'Smart contract for trading assets', |
There was a problem hiding this comment.
| description: 'Smart contract for trading assets', | |
| description: 'Smart Contract for asset transfer ledger sample', |
It would be better to change the description to match the sample.
| appraisedValue, | ||
| }; | ||
|
|
||
| await ctx.stub.putState(assetID, Buffer.from(JSON.stringify(asset))); |
There was a problem hiding this comment.
Although it is not essential, in order to maintain JSON consistency, it seems better to use json-stringify-deterministic and sort-keys-recursive when inserting data, in the same way as asset-transfer-basic.
Please refer to here.
| asset = JSON.parse(historyRecord.value.toString()) as Asset; | ||
| } | ||
| // Convert the timestamp to a Date object. | ||
| const seconds = historyRecord.timestamp.seconds; |
There was a problem hiding this comment.
Timestamp includes nanos as a field, so I think it is necessary to convert this to Date as well.
interface Timestamp {
seconds: Long;
nanos: number;
}
This commit implements the TypeScript version of the asset-transfer-ledger-queries Chaincode.
Resolves #1232