Fixing case where signingJobs is empty for non-DERIVE_KEY_AND_SIGN requests. #139
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
remotesigning/remote_signing.go
Outdated
| @@ -58,31 +58,30 @@ func GraphQLResponseForRemoteSigningWebhook( | |||
| webhook webhooks.WebhookEvent, | |||
| seedBytes []byte, | |||
| ) (SigningResponse, error) { | |||
| // Calculate the xpub for each L1 signing job | |||
| // If derive key and sign, calculate the xpub for each L1 signing job | |||
| subEventTypeStr := (*webhook.Data)["sub_event_type"].(string) | |||
There was a problem hiding this comment.
This type assertion could panic if webhook.Data is nil or if sub_event_type isn't a string. Consider using a safer type assertion with error checking:
subEventTypeStr, ok := (*webhook.Data)["sub_event_type"].(string)
if !ok {
return nil, fmt.Errorf("sub_event_type not found or invalid type")
}Spotted by Graphite Reviewer
Is this helpful? React 👍 or 👎 to let us know.
There was a problem hiding this comment.
Should I add this? I think this is the comment that caused the previous failure
There was a problem hiding this comment.
no, this one is different, it checks the existence of sub_event_type
There was a problem hiding this comment.
gotcha, will do.
There was a problem hiding this comment.
well, technically it's fine because we send the webhook. It's up to you.
There was a problem hiding this comment.
Guess doesn't hurt to add if someone is debugging this later
JasonCWang
force-pushed
the
11-26-fixing_case_where_signingjobs_is_empty_for_non-derive_key_and_sign_requests
branch
from
00cc9c6 to
f7a633c
Compare
Merge activity
|
JasonCWang
deleted the
11-26-fixing_case_where_signingjobs_is_empty_for_non-derive_key_and_sign_requests
branch
No description provided.