-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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
Add private data go application #1264
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.
Thank you for a great contribution! A few suggested changes in-line. Also there is a logic error that definitely needs to be corrected.
The issue seems to be the JSON mapping with property names appearing with initial capital letters (as they appear in the Go structs) whereas the contract is working with initial lower case letters. The JSON mappings should probably be explicitly specified throughout the code to be safe.
@@ -0,0 +1,358 @@ | |||
/* | |||
Copyright 2022 IBM All Rights Reserved. |
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.
Since this is new code, I would use 2024 as the year
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.
Thx! Corrected.
fmt.Println("~~~~~~~~~~~~~~~~ As Org1 Client ~~~~~~~~~~~~~~~~") | ||
|
||
// Create new assets on the ledger. | ||
createAssets(*contractOrg1) |
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 would avoid dereferencing the Contract pointer here. Instead, pass the pointer with the createAssets
function taking a *client.Contract
argument
createAssets(*contractOrg1) | |
createAssets(contractOrg1) |
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.
Thx! Corrected.
createAssets(*contractOrg1) | ||
|
||
// Read asset from the Org1's private data collection with ID in the given range. | ||
getAssetByRange(*contractOrg1) |
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.
Avoid dereferencing and pass the pointer
getAssetByRange(*contractOrg1) | |
getAssetByRange(contractOrg1) |
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.
Done.
err = transferAsset(*contractOrg1, assetID1) | ||
if err == nil { | ||
doFail("TransferAsset transaction succeeded when it was expected to fail") | ||
} |
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.
You could condense this slightly but either form is valid so your choice. I would suggest passing the pointer rather than copying the value though:
err = transferAsset(*contractOrg1, assetID1) | |
if err == nil { | |
doFail("TransferAsset transaction succeeded when it was expected to fail") | |
} | |
if err := transferAsset(contractOrg1, assetID1); err == nil { | |
doFail("TransferAsset transaction succeeded when it was expected to fail") | |
} |
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.
Can't use the condensed form because I need to print the error after the if statement. Removed dereference.
fmt.Println("\n~~~~~~~~~~~~~~~~ As Org2 Client ~~~~~~~~~~~~~~~~") | ||
|
||
// Read the asset by ID. | ||
readAssetByID(*contractOrg2, assetID1) |
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.
Avoid dereference
readAssetByID(*contractOrg2, assetID1) | |
readAssetByID(contractOrg2, assetID1) |
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.
Done.
fmt.Printf("*** Result: %s\n", result) | ||
} | ||
|
||
func transferAsset(contract client.Contract, assetID string) (err error) { |
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.
func transferAsset(contract client.Contract, assetID string) (err error) { | |
func transferAsset(contract *client.Contract, assetID string) (err error) { |
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.
Done.
return | ||
} | ||
|
||
func deleteAsset(contract client.Contract, assetID string) (err error) { |
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.
func deleteAsset(contract client.Contract, assetID string) (err error) { | |
func deleteAsset(contract *client.Contract, assetID string) (err error) { |
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.
Done.
return | ||
} | ||
|
||
func purgeAsset(contract client.Contract, assetID string) (err error) { |
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.
func purgeAsset(contract client.Contract, assetID string) (err error) { | |
func purgeAsset(contract *client.Contract, assetID string) (err error) { |
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.
Done.
return | ||
} | ||
|
||
func readAssetPrivateDetails(contract client.Contract, assetID, collectionName string) bool { |
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.
func readAssetPrivateDetails(contract client.Contract, assetID, collectionName string) bool { | |
func readAssetPrivateDetails(contract *client.Contract, assetID, collectionName string) bool { |
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.
Done.
dataForAgreement := struct { | ||
AssetID string | ||
AppraisedValue int | ||
}{assetID, 100} |
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 seems to be the cause of the transfer failure. The agreement is hashed and is sensitive to the capitalization of the JSON property names, which the contract treats with an initial lower-case letter
dataForAgreement := struct { | |
AssetID string | |
AppraisedValue int | |
}{assetID, 100} | |
dataForAgreement := struct { | |
AssetID string `json:"assetID"` | |
AppraisedValue int `json:"appraisedValue"` | |
}{assetID, 100} |
func transientData(key string, value any) map[string][]byte { | ||
valueAsBytes, err := json.Marshal(&value) | ||
if err != nil { | ||
panic(err) | ||
} | ||
|
||
return map[string][]byte{key: valueAsBytes} | ||
} |
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 definitely does the job for this use-case, where the application only creates transient maps with a single key/value. I wonder if (as an example) it might be worth using a more generic alternative, something like:
func marshal(value any) []byte {
result, err := json.Marshal(value)
if err != nil {
panic(err)
}
return result
}
This would be used as follows:
client.WithTransient(map[string][]byte{
"asset_properties": marshal(asset1Data),
})
Just an idea to consider.
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.
Yes, good point and good idea. I would add a type alias to the map so it looks a bit nicer:
type transient = map[string][]byte
// ...
client.WithTransient(transient{
"asset_properties": marshal(asset1Data),
}),
wdyt?
result := formatJSON(resultBytes) | ||
if result == "" { | ||
fmt.Println("*** No result") | ||
return false | ||
} |
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.
If the transaction result is empty, I think trying to parse it as JSON will fail. Instead if checking the the JSON being empty, I think you will need to check for empty resultBytes
:
result := formatJSON(resultBytes) | |
if result == "" { | |
fmt.Println("*** No result") | |
return false | |
} | |
if len(resultBytes) == 0 { | |
fmt.Println("*** No result") | |
return false | |
} | |
result := formatJSON(resultBytes) |
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.
Yes, you're right. Thank you for pointing out. Fixed it in all places.
205bf39
to
ddef44a
Compare
Thank you for the review. I fixed the issue and added all the changes. You can take another look. |
Created project directory, app.go and connect.go files. Reused the logic for connect.go from the events application and added second organization setup. Implemented private data transaction example in go as described in the main documentation in "Tutorials/Using Private Data in Fabric". Updated README.md with the command to run the go application and the script which runs the application in the Github Actions workflow. Fixed typos and punctuation in the private data typescript application. Signed-off-by: Stanislav Jakuschevskij <stas@two-giants.com>
ddef44a
to
e6aca84
Compare
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.
Thank you for the updates. This looks good to me!
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.
Nice job, thanks!
Private Data Go Application
This PR contains an implementation of a go application using Hyperledger Fabrics private data feature as described in the main Hyperledger Fabric documentation here.
It follows the exact same procedure and uses the same naming as the private data typescript sample here.
The implementation uses the Gateway SDK and the issue reference is here in the Fabric Gateway SDK repository.
Summary