feat: Invalidate cached signature on new signing request#199
feat: Invalidate cached signature on new signing request#199BranimirTomeljak wants to merge 6 commits into
Conversation
|
Go Test coverage is 37.1 %\ ✨ ✨ ✨ |
|
Go Test coverage is 37.3 %\ ✨ ✨ ✨ |
|
Go Test coverage is 37.2 %\ ✨ ✨ ✨ |
mpetrun5
left a comment
mpetrun5
left a comment
There was a problem hiding this comment.
I think it is better to invalidate the old signature and remove it from cache so it is not returned until the new one is generated.
BranimirTomeljak
changed the title
|
Go Test coverage is 36.9 %\ ✨ ✨ ✨ |
|
Go Test coverage is 36.9 %\ ✨ ✨ ✨ |
| s.Equal(http.StatusBadRequest, recorder.Code) | ||
| } | ||
|
|
||
| func (s *SigningHandlerTestSuite) Test_HandleSigning_ErrorHandlingMessage() { |
There was a problem hiding this comment.
Why are these tests deleted instead of just adding a new test?
| return nil | ||
| } | ||
|
|
||
| func sessionKey(protocol ProtocolType, chainID uint64, depositID string) string { |
There was a problem hiding this comment.
Can we move this function somewhere?
Maybe under TSS signing and reuse it the protocol handlers so it is consistent.
|
Go Test coverage is 36.9 %\ ✨ ✨ ✨ |
|
Out of curiosity, why not store deadline along with signature and return cached if it's still valid (deadline hasn't passed)? |
It would have to be a breaking change and I would like to avoid that because you would need to ask for a specific nonce/deadline signature pair instead of just sending the ID of the order (could be done if some logic to track and expire deadlines is added so you either get that one or generate a new one). This is more of an edge case when someone calls this twice for the same order (probably because the first try failed) which shouldn't even happen as the solver can just keep the first signature if it is in a hurry. |
3 participants
Description
A requeued borrow fill signs a fresh deadline each attempt, but the MPC signature cache was keyed by chain and deposit id only. The retry's status subscription returned the first attempt's cached signature, which no longer matched the new request.
Related Issue Or Context
Closes: #
How Has This Been Tested? Testing details.
Types of changes
Checklist: