search

zkemail / email-wallet

A smart contract wallet controlled using email
https://emailwallet.org
MIT License
96 stars 15 forks source link

feat: return error email #48

Closed Bisht13 closed 5 months ago

Bisht13 commented 5 months ago

Fixes #40

gitguardian[bot] commented 5 months ago

️✅ There are no secrets present in this pull request anymore.

If these secrets were true positive and are still valid, we highly recommend you to revoke them. Once a secret has been leaked into a git repository, you should consider it compromised, even if it was deleted immediately. Find here more information about risks.

🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.

Our GitHub checks need improvements? Share your feedbacks!

SoraSuegami commented 5 months ago

Thank you for your PR! I think your codes are good! Could you just modify the type of original_subject field in the Error to Option<String> because we also need to send error emails for account initialization emails? https://github.com/zkemail/email-wallet/pull/48/files#diff-625da5a352196440c9ac90f8f65dd8b1c1d6eb225c755c395317375b849a3851R57

Also, after merging this PR, we next need to define the specific error types using thiserror crate rather than specifying errors by raw strings in the error_email_if_needed function.

Bisht13 commented 5 months ago

Thank you for your PR! I think your codes are good! Could you just modify the type of original_subject field in the Error to Option<String> because we also need to send error emails for account initialization emails? https://github.com/zkemail/email-wallet/pull/48/files#diff-625da5a352196440c9ac90f8f65dd8b1c1d6eb225c755c395317375b849a3851R57

Also, after merging this PR, we next need to define the specific error types using thiserror crate rather than specifying errors by raw strings in the error_email_if_needed function.

Implemented the requested changes.

SoraSuegami commented 5 months ago

Thank you for your update!

https://github.com/zkemail/email-wallet/pull/48/files#diff-625da5a352196440c9ac90f8f65dd8b1c1d6eb225c755c395317375b849a3851R305-R308 Here, could you add the original subject only if it is not None, instead of unwrapping it?