Add initial client go sdk prototype #452
Conversation
|
Fabric go sdk PR was already merged! |
mbrandenburger
changed the title
There was a problem hiding this comment.
In general looks very nice regarding the user-facing API. I have a number of observations regarding implementation but most of the issue might be better addressed elsewhere (primary concern is that there are some issues like interface between c and go (issue #412) and encoding/decoding, formats of request/response we seem to make somewhat implicit which i think we should make a bit more explcit/consciously ...). For actual PR, assuming you are happy with my clickable suggestions, themain concern is that it doesn't build
internal/utils/utils.go
Outdated
| "crypto/sha256" | ||
| "encoding/hex" |
There was a problem hiding this comment.
These and below internal/protos cause me build problems until removed ...
| credentials := &protos.Credentials{} | ||
| err = proto.Unmarshal(credentialsBytes, credentials) | ||
| if err != nil { | ||
| return fmt.Errorf("cannot unmarshal credentials: %s", err) | ||
| } | ||
| log.Printf("Received credentials from enclave: %s\n", credentials) | ||
|
|
||
| // perform attestation evidence transformation | ||
| credentials, err = attestation.ToEvidence(credentials) | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| credentialsBytes = protoutil.MarshalOrPanic(credentials) | ||
| credentialsBase64 := base64.StdEncoding.EncodeToString(credentialsBytes) |
There was a problem hiding this comment.
Could be worth "outsourcing" this in a separate function as a simply cmd-line utility wrapper would be exactly the transform you would need in the peer cli (i.e., only a single and simple implementation and also a simplification of the peer cli script ...
| package fpc | ||
|
|
||
| func Encrypt(input []byte, encryptionKey []byte) ([]byte, error) { | ||
| return input, nil | ||
| } | ||
|
|
||
| func KeyGen() ([]byte, error) { | ||
| return []byte("fake key"), nil | ||
| } | ||
|
|
||
| func Decrypt(encryptedResponse []byte, resultEncryptionKey []byte) ([]byte, error) { | ||
| return encryptedResponse, nil | ||
| } |
There was a problem hiding this comment.
So you assume we surface pdo crypto via cgo? As discussed in #412, i thought conceptually bridging at a higher-level (for arguments discussed there). The devil is in the details and what i propose might not work. However, i feel we should make a conscious decision on this interface. Right now it seems to me we end up making an implicit decision while not necessarily have considered the implication (maybe you did, but then i think we should reflect it in #412 and also resolve it there?)
To some extent it also seems to related on what exactly the semantics is of the "Transaction Crypto Library" boxes in mural?
I agree that these could be better addressed elsewhere, and I would add later after getting the next release out. |
If we don't agree on these encodings, interfaces and types (either explicitly or impliciltly) then there will be no next release? |
Interfaces and protobufs are already defined, there is no need (at least at this point) for additional details -- hopefully this matches with your definition of "implicit" agreement. |
The interfaces i was referring to is regarding #421 which we haven't designed. Regarding protobufs, we forgot to define message format for (encrypted) fpc chaincode invocations (request and response). Our spec also omitted the encoding of messages between client and fpc_stub. We agreed in yesterday's call verbally on that encoding (i.e., base64-encoding of binary-serialization of protobuf objects), my point was only that we might also simply document that (and remove the misleading reference to json-protobuf). Anyway, i can do an AR on myself to create a PR which creates a proposal for the missing protobufs and does the small fix to interfaces.md regarding the encoding. |
…o deal Signed-off-by: michael steiner <[email protected]>
mbrandenburger
force-pushed
the
go-sdk
branch
from
eecb1cd to
e306ea6
Compare
- initial go sdk extension for FPC that hides the FPC transaction flow from the user - integrate with test-network - CreateEnclave requires target peer endpoint - FPC invocations are invoked (queried) at a defined endpoint - Update go sdk version and address review Signed-off-by: bur <[email protected]> Co-authored-by: Michael Steiner <[email protected]>
mbrandenburger
force-pushed
the
go-sdk
branch
from
6548215 to
f95c5e5
Compare
Signed-off-by: bur [email protected]
What this PR does / why we need it:
from the user
Which issue(s) this PR fixes:
Fixes #289
Special notes for your reviewer:
Does this PR introduce a user-facing changes and/or breaks backward compatability?: