Closed Tangelo1 closed 4 years ago
This pull request introduces 4 alerts when merging fcf4057ffde7a85f8298eadedf6c3199f045747d into 27003be38beb254090e7708c4e45c99f42303571 - view on LGTM.com
new alerts:
This pull request introduces 4 alerts when merging 242faf320e3a0534e561975dc2a21ac7c9716cab into 27003be38beb254090e7708c4e45c99f42303571 - view on LGTM.com
new alerts:
@smithbk With regard to your trust ping comments -- I think the did-exchange complete would be sufficient to test the connection, but I'm wondering what we should do in the case that the AUT doesn't want to use did-exchange and chooses to use the connections protocol instead. I think in this case, without something like a trust ping there'd be no real way to verify the connection. Open to ideas for this, but I had originally put in the trust pings following how it's done by the current code. I've removed them for now, but I think we definitely need a method to verify the connections. Also I've re-read the OOB spec and it doesn't seem there's an explicit way to discover features in it.
@smithbk With regard to the message prefixes. I've centralized the prefix definitions into the Suite
class. When the time comes we can make these types such that the Suite.TYPE_PREFIX
and Suite.ALT_TYPE_PREFIX
to be the same. This effectively makes the suite only support the one prefix.
@Tangelo1 IIRC, the connection protocol has an ACK message which would be equivalent to the DID exchange complete message. WRT the current code, yeh, I think an early version of the connection protocol may have said or recommended using the trust ping protocol to complete the connection. Also, the trust ping protocol used to be part of Aries Profile V1 but was removed. Obviously APTS wasn't updated appropriately.
Yeh, I was misremembering about OOB having features. If you wanted to do a trust ping, I think the proper way is that the trust ping request is added to the OOB message as an attachment. The receiver of the OOB message would optionally process the attachment and send the trust ping response. That said, I don't think it is necessary. Given that we could use the ACK message for the connection protocol and the complete message for the DID exchange protocol, I think it is sufficient to verify that the connection was successful.
@smithbk Yep, you're correct. I forgot about the ACK. I agree that should be sufficient to test the connection.
Summary of changes:
handshake_protocol
list.https://didcomm.org/
in their@type
field instead ofdid:sov:BzCbsNYhMrjHiqZDTUASHg;spec/
did:sov:BzCbsNYhMrjHiqZDTUASHg;spec/
is still supported however.NOTES: