Skip to content
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

dev/review notes #8

Closed
wants to merge 1 commit into from
Closed

Conversation

livingrockrises
Copy link
Contributor

@livingrockrises livingrockrises commented Sep 11, 2024

  1. Should make list of all positive and negative test cases we are planning to add.
  2. Add accounting test cases
  3. Run this on fork of some mainnet using real feeds
  4. Run swapping function on fork of some mainnet

@livingrockrises livingrockrises marked this pull request as draft September 11, 2024 08:42
@@ -44,15 +44,22 @@ contract BiconomyTokenPaymaster is
using SignatureCheckerLib for address;

// State variables

// Review: where will the be used since we have pre charge and refund model?
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can just discuss here on each points by starting a comment like this
@ShivaanshK

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree. FeeCollector is unnecessary.

@livingrockrises livingrockrises deleted the chore/review-view branch October 7, 2024 15:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants