DIDComm v1 interfaces like those available for v2 #39
Conversation
Signed-off-by: Daniel Bluhm <[email protected]>
instead of through the old crypto module Signed-off-by: Daniel Bluhm <[email protected]>
Signed-off-by: Daniel Bluhm <[email protected]>
Signed-off-by: Daniel Bluhm <[email protected]>
Signed-off-by: Daniel Bluhm <[email protected]>
DIDComm v1 isn't quite legacy yet Signed-off-by: Daniel Bluhm <[email protected]>
Signed-off-by: Daniel Bluhm <[email protected]>
Signed-off-by: Daniel Bluhm <[email protected]>
| """Result of unpacking a message.""" | ||
|
|
||
| unpacked: bytes | ||
| encrytped: bool |
| return peer_4_pattern_short.match(did) or peer_4_pattern_long.match(did) | ||
| return bool(peer_4_pattern_short.match(did)) or bool( | ||
| peer_4_pattern_long.match(did) | ||
| ) | ||
|
|
There was a problem hiding this comment.
Would surrounding the entire or in the bool be cleaner and/or make more sense to someone reading the code?
| enc_sender = None | ||
| nonce = None | ||
| enc_cek = crypto_box.crypto_box_seal(target_xk, cek_b) | ||
| builder.add_recipient( | ||
| JweRecipient(encrypted_key=enc_cek, header={"kid": target_vk.kid}) | ||
| ) |
There was a problem hiding this comment.
Nonce & enc_sender set but not used?
There was a problem hiding this comment.
Is this just to "zero out" the values? If so, wouldn't it make more sense to have it outside the if statement?
There was a problem hiding this comment.
Used to be used, refactored and then it was no longer used. Good catch!
| @property | ||
| def key(self) -> str: | ||
| """Return base58 encoded key.""" | ||
| return base58.b58encode(self.value).decode() | ||
|
|
||
| @property | ||
| def kid(self) -> str: | ||
| """Get the key ID.""" | ||
| return self.key |
There was a problem hiding this comment.
Would this hold true for the kid if the did was did:peer:1/2 during a DIDComm V1 exchange?
| # Legacy formats | ||
| # V1 formats |
There was a problem hiding this comment.
I believe that Legacy here referred to something else besides DIDComm V1
| class V1UnpackResult(NamedTuple): | ||
| """Result of unpacking.""" | ||
|
|
||
| message: bytes | ||
| recip: str | ||
| sender: Optional[str] |
There was a problem hiding this comment.
Should message here load the JSON to match the interface for DIDComm V2?
There was a problem hiding this comment.
In this case, this is a "lower level" result object. I'll rename to make it's function clearer.
| if not isinstance(seed, bytes): | ||
| raise ValueError("Seed value is not a string or bytes") | ||
| if len(seed) != 32: | ||
| raise ValueError("Seed value must be 32 bytes in length") |
There was a problem hiding this comment.
Won't this fail if it's a str? Won't a str's len() value also be off if unicode (particularly CJK) is used?
Method signature signifies that only bytes is allowed, but error says that str is as well.
| @dataclass | ||
| class V1UnpackResult: | ||
| """Result of unpacking a message.""" | ||
|
|
||
| unpacked: bytes | ||
| encrytped: bool | ||
| authenticated: bool | ||
| recipient_kid: str | ||
| sender_kid: Optional[str] = None | ||
|
|
||
| @property | ||
| def message(self) -> dict: | ||
| """Return unpacked value as a dict. | ||
|
|
||
| This value is used to preserve backwards compatibility. | ||
| """ | ||
| return json.loads(self.unpacked) |
There was a problem hiding this comment.
This class appears to have the same name as a NamedTuple in v1/base.py, leading to some confusion
| @@ -1,31 +1,17 @@ | |||
| """Test Pack and Unpack.""" | |||
There was a problem hiding this comment.
Does this file need to stay here, or move to ../v1/?
There was a problem hiding this comment.
Should stay here; I'm treating the crypto module in the legacy folder as a "do not touch, backwards compat only" thing since it went out in a release already.
There was a problem hiding this comment.
And it mimics exactly the dependency it replaces, aries-staticagent-python
| """Result of unpacking a message.""" | ||
|
|
||
| unpacked: bytes | ||
| encrytped: bool |
There was a problem hiding this comment.
Maybe this isn't a typo?
This PR adds interfaces for DIDComm v1 like the ones we have for v2.
The purpose of this addition is to help users of the library implement both v1 and v2 in the same code base more easily by providing a (relatively) consistent interface between v1 and v2. There are, of course, differences arising from the differences in the two versions but the style and patterns should be familiar at least.