hyperledger-archives / aries-framework-go

Hyperledger Aries Framework Go provides packages for building Agent / DIDComm services.
https://wiki.hyperledger.org/display/ARIES/aries-framework-go
Apache License 2.0
241 stars 158 forks source link

UX issues with `client.introduce` #937

Open llorllale opened 4 years ago

llorllale commented 4 years ago

These are some issues I found while attempting to understand usage of the client.introduce package as of commit b0fa7cbc62c2e75ac9d8d13b1ae163e9b61a009e. You might call these "aesthetic" issues since they are not currently inhibiting functionality per se.

I'm attaching here two snippets of code: one is my attempt to understand the current implementation, and the other is a sketch of what I think the API should look like.

Current

```go package example_introduce import ( "encoding/json" "errors" "github.com/hyperledger/aries-framework-go/pkg/client/introduce" "github.com/hyperledger/aries-framework-go/pkg/didcomm/common/service" "github.com/hyperledger/aries-framework-go/pkg/didcomm/protocol/didexchange" intsvc "github.com/hyperledger/aries-framework-go/pkg/didcomm/protocol/introduce" "github.com/hyperledger/aries-framework-go/pkg/framework/aries" ) func main() { aries, err := aries.New() defer aries.Close() ctx, err := aries.Context() client, err := introduce.New(ctx, &didexchange.Invitation{}) // why do we need to provide an invitation here? events := make(chan service.DIDCommAction) err := client.RegisterActionEvent(events) for event := range events { switch event.Message.Header.Type { case intsvc.RequestMsgType: // inspect request req := &intsvc.Request{} err := json.Unmarshal(event.Message.Payload, req) // prepares data // HandleRequest accepts a service.DIDCommMsg. Should I just pass in the one I received? client.HandleRequest(*event.Message, &service.Destination{}) // or // prepares data skip proposal // HandleRequest accepts a service.DIDCommMsg. Should I just pass in the one I received? client.HandleRequestWithInvitation(*event.Message, &didexchange.Invitation{}) // what should I pass to Continue() here in order to make it so the framework // can reply with a proposal? event.Continue(client.InvitationEnvelope(event.Message.Header.Thread.ID)) } } } ```

Desired

```go package example_introduce import ( "encoding/json" "errors" "github.com/hyperledger/aries-framework-go/pkg/client/introduce" "github.com/hyperledger/aries-framework-go/pkg/didcomm/common/service" "github.com/hyperledger/aries-framework-go/pkg/didcomm/protocol/didexchange" intsvc "github.com/hyperledger/aries-framework-go/pkg/didcomm/protocol/introduce" "github.com/hyperledger/aries-framework-go/pkg/framework/aries" ) // In this exercise I did not need: // - client.HandleRequest() // its semantics aren't clear-cut because it accepts a DIDCommMsg. also it's saving an invitationenvelope for no clear reason. // - client.HandleRequestWithInvitation() // same points as client.HandleRequest() apply. // - client.InvitationEnvelope() // I don't see the need for the end-user-developer to use this function. // - client.SendProposal() // * when introducer starts protocol: substituted by "client.Introduce()" in my example below, with a different signature // * when introducer receives 'request': substituted by "introduce.WithCandidate()" as arg to event.Continue() in my example below // - client.SendProposalWithInvitation() // proposal messages don't contain invitations. func main() { aries, err := aries.New() defer aries.Close() ctx, err := aries.Context() client, err := introduce.New(ctx, &didexchange.Invitation{}) // why do we need to provide an invitation here? events := make(chan service.DIDCommAction) err := client.RegisterActionEvent(events) for event := range events { switch event.Message.Header.Type { case intsvc.RequestMsgType: // only introducer receives 'request' msgs // inspect request req := &intsvc.Request{} err := json.Unmarshal(event.Message.Payload, req) // introducer can either accept request or reject it... // for accepting, we have a gap: framework needs to provide functionality for the semantic layer to // match a candidate for this request. For now, we'll just assume this functionality is there... // accept (assuming we have a matching candidate): // WithCandidate() is a made up function. An alternative argument to WithCandidate(did) can be a connectionID. // The framework should ideally then transparently: // 1. look up that candidate in our storage and determine their destination // 2. look up the metadata about the sender of the 'request' msg and combine that with the request's details // to form a suitable proposal msg (again, ideal scenario) // 3. send proposal to candidate and change state to 'arranging' event.Continue(introduce.WithCandidate("did:example:candidate")) // reject event.Stop(errors.New("sender not allowed to request for introductions")) case intsvc.ProposalMsgType: // only introducees receive 'proposal' msgs // inspect proposal prop := &intsvc.Proposal{} err := json.Unmarshal(event.Message.Payload, prop) // introducee can either accept proposal or reject it... // to accept: // WithInvitation() is a made up function. // The invitation is optional as per the RFC. // To continue without invitation just do: event.Continue(nil) // In either case, the 'approval' attribute is set to 'true' in the response. event.Continue(introduce.WithInvitation(&didexchange.Invitation{})) // to reject: // The 'approval' attribute is set to 'false' in the response event.Stop(errors.New("the introducee does not like this proposal")) case intsvc.ResponseMsgType: // only introducer receives 'response' msgs. // I think it most likely that the introducer's state machine should just auto accept these // responses and continue. These are responses to proposals sent out by the Introducer // anyway. // inspect response resp := &intsvc.Response{} err := json.Unmarshal(event.Message.Payload, resp) // these responses are reactions to proposals sent out by the Introducer because they either: // 1. unilaterally started the introduce protocol // 2. reacted to an incoming 'request' // 1. unilaterally starting the protocol: // Continuing the flow here means the framework forwards the invitations contained in this // response (if any) to all other introducees. // 2. reacting to incoming 'request': // Continuing the flow here means the framework forwards the invitation in the response to // the agent that sent the 'request' msg. // // No arguments should be required by event.Continue() to proceed in either case. event.Continue(nil) // ... or reject?: event.Stop(errors.New("?")) } } // As an introducer I can unilaterally start the introduce protocol with proposals: // Introduce() is a made up function. It introduces all given parties to each other. // ideally, the framework should look up their respective connection records and build // suitable proposals and determine all destinations. // In this example with three introducees the framework will perform 3 introductions, all within // a single instance of the Introduce state machine. // IF the introducer has not enabled 'AutoAccept' (for responses at least) then they should listen // for 'response' msgs in the event stream above. client.Introduce( "did:example:123", "did:example:456", "did:example:789", ) // As an introducee I can unilaterally start the introduce protocol with a 'request': err := client.SendRequest(&service.Destination{}) // missing details of the request (eg. criteria) } ```

troyronda commented 4 years ago

Agree.