Cleaned up the response verification #649
Conversation
chain-signatures/contract/src/lib.rs
Outdated
| @@ -117,6 +117,8 @@ impl MpcContract { | |||
| fn add_sign_result(&mut self, payload: &SignatureRequest, signature: SignatureResponse) { | |||
| if self.pending_requests.contains_key(payload) { | |||
| self.pending_requests.insert(payload, &Some(signature)); | |||
| } else { | |||
| env::panic_str(&format!("Key not found: {:?}", payload)) | |||
There was a problem hiding this comment.
What is the node behavior in case of a failure?
There was a problem hiding this comment.
This needs a test, I'll add it in
There was a problem hiding this comment.
The signature is discarded after retying, is that alright?
There was a problem hiding this comment.
if it's discarded, we need to alert the requestor of the signature that it failed to publish somehow. I guess with yield/resume we can do this by returning an error and pancing so we can do this in future, but maybe for now we just store the signature to be sent again on the next iteration
There was a problem hiding this comment.
For now we can add a record_failure method to contract, and cross contract call to it when it fails. The record_failure can be actually not record anything on chain, but because it is cross contract called, we can modify the indexer part to catch it.
Of course yield/resume would be better if it can be implemented soon, and signature discarded as a short term solution is acceptable.
There was a problem hiding this comment.
I ended up pushing it to the back of the queue and retrying unless it fails in the contract. It's easy to DoS the contract if you allow incorrect responses to cancel the request.
chain-signatures/contract/src/lib.rs
Outdated
| @@ -117,6 +117,8 @@ impl MpcContract { | |||
| fn add_sign_result(&mut self, payload: &SignatureRequest, signature: SignatureResponse) { | |||
| if self.pending_requests.contains_key(payload) { | |||
| self.pending_requests.insert(payload, &Some(signature)); | |||
| } else { | |||
| env::panic_str(&format!("Key not found: {:?}", payload)) | |||
There was a problem hiding this comment.
if it's discarded, we need to alert the requestor of the signature that it failed to publish somehow. I guess with yield/resume we can do this by returning an error and pancing so we can do this in future, but maybe for now we just store the signature to be sent again on the next iteration
DavidM-D
force-pushed
the
dmd/cleanup-signature-check
branch
2 times, most recently
from
46dec87 to
371bc62
Compare
previously we'd stop publishing if we had an error
DavidM-D
force-pushed
the
dmd/cleanup-signature-check
branch
from
371bc62 to
6867939
Compare
|
I think this is ready to merge |
Terraform Feature Environment Destroy (dev-649)Terraform Initialization ⚙️
|
This now does native ecdsa key derivation and the epsilon is unreversed