refactor: Remove template in favor of an enum for signer#454
refactor: Remove template in favor of an enum for signer#454mergify[bot] merged 9 commits intomasterfrom
Conversation
Going over the spec plus the identity designs so far, it seems we shouldn't expect more than 1-2 more request types worst case, if any. This was discussed with Hans and Enzo at some point last week. While it makes the construction a bit bulkier it removes the generic and as a result removes one dependency. Also I think this might make it easier to modify the trait or future related trait realizations (implementations).
eftychis
force-pushed
the
eftychis-minor-signer
branch
from
902ecf9 to
eae811b
Compare
| // let mut sender = vec![0; 32]; | ||
| // sender.push(0x02); | ||
| // let sender = Blob::from(sender); | ||
| // let request_with_sender = MessageWithSender { request, sender }; |
There was a problem hiding this comment.
Why is this code commented instead of removed and the Bug() above replaced with the new Principal type you just added?
There was a problem hiding this comment.
hmm .. Still getting timeouts with using directly serialize_bytes. Let me see what is produced and compare request ids.
There was a problem hiding this comment.
Re-ping. As mentioned, the example #450, also experiences this (You get
AgentError(
# TimeoutWaitingForResponse,
# )
(which is due to request id calculation not being the same.)
)
In both cases custom serialization seems fine to me (we implement a serializer). I tried both current and the bytes and the blob serializer and for all cases we got a deviation with the handler when we request status. (I will look more onto this perhaps today or Monday and do some tracing and proper debugging; but I am seeing the same behavior in both.)
P.S. Removed the lines showing the problem above.
eftychis
changed the title
hansl
left a comment
hansl
left a comment
There was a problem hiding this comment.
Do we still need the 'a lifetime on sign?
|
On the top of my head I think it is because of the argument, but let me check. |
## Changelog for common: Branch: master Commits: [dfinity-lab/common@56b6f6f3...87104afb](https://github.com/dfinity-lab/common/compare/56b6f6f3b0e7cf26a8eaf75b2a1ac118c827538d...87104afb2aa76f1cf8f6709328c1a2116f273147) * [`87104afb`](https://github.com/dfinity-lab/common/commit/87104afb2aa76f1cf8f6709328c1a2116f273147) Added rrkapitz aka Ruediger Kapitza ([dfinity-lab/common#454](http://r.duckduckgo.com/l/?uddg=https://github.com/dfinity-lab/common/issues/454))
## Changelog for common: Branch: master Commits: [dfinity-lab/common@56b6f6f3...87104afb](https://github.com/dfinity-lab/common/compare/56b6f6f3b0e7cf26a8eaf75b2a1ac118c827538d...87104afb2aa76f1cf8f6709328c1a2116f273147) * [`87104afb`](https://github.com/dfinity-lab/common/commit/87104afb2aa76f1cf8f6709328c1a2116f273147) Added rrkapitz aka Ruediger Kapitza ([dfinity-lab/common#454](http://r.duckduckgo.com/l/?uddg=https://github.com/dfinity-lab/common/issues/454))
2 participants
Going over the spec plus the identity designs so far, it seems we
shouldn't expect more than 1-2 more request types worst case, if
any. This was discussed with Hans and Enzo at some point last week.
While it makes the construction a bit bulkier it removes the generic
and as a result removes one dependency. Also I think this might make
it easier to modify the trait or future related trait
realizations (implementations).