-
Notifications
You must be signed in to change notification settings - Fork 262
feat: credential proxy deposit pool #5945
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
mmsinclair
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good for a 1st step that we can iterate from
Added some comments for @benedettadavico as a reminder to add monitoring on logs to look for critical errors and notify us
| let ed25519_keypair = ed25519::KeyPair::new(&mut rng); | ||
| ) -> Result<Vec<WalletShare>, CredentialProxyError> { | ||
| // don't proceed if we don't have quorum available as the request will definitely fail | ||
| if !state.quorum_available() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this future proofing, or something that it detects somewhere?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just an early escape hatch. if we don't have a quorum, there's not a chance we'll succeed in obtaining threshold number of shares
| // that one is tricky. deposits technically got made, but we somehow failed to parse response, | ||
| // in this case terminate the proxy with 0 exit code so it wouldn't get automatically restarted | ||
| // because it requires some serious MANUAL intervention | ||
| error!("CRITICAL FAILURE: failed to parse out deposit information from the contract transaction. either the chain got upgraded and the schema changed or the ecash contract got changed! terminating the process. it has to be inspected manually. error was: {err}"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@benedettadavico reminder to add some monitoring to watch for this
| } | ||
|
|
||
| async fn check_quorum_state(&self) -> Result<bool, CredentialProxyError> { | ||
| let client_guard = self.client.query_chain().await; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, I see now where this check is done (from comment above)
baef659 to
ea7c0cd
Compare
NET-486
this PR changes behaviour of the credential proxy. rather than making the ecash deposits on demand, it will now hold a pool of available deposits. furthermore it will monitor for quorum state. this will reduce wastage in few ways:
This change is