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

Add refunds #75

Merged
merged 6 commits into from
Dec 16, 2021
Merged

Add refunds #75

merged 6 commits into from
Dec 16, 2021

Conversation

kerzhner
Copy link
Contributor

@kerzhner kerzhner commented Dec 15, 2021

A customer can refund a ticket after the authorization window expires. When refund is called for a ticket:

  1. A batch is created with all tickets between the last authorized ticket (exclusive) and the ticket being refunded (inclusive).
  2. All tickets in the batch are refunded.

This PR also reduces BatchStatus to 2 types: Authorized and Withdrawn.

Fixes #74

@kerzhner kerzhner force-pushed the refund branch 3 times, most recently from 8d55c1f to 13643ae Compare December 15, 2021 14:09
Base automatically changed from fraud to main December 15, 2021 23:25
At the moment, L2 contract is not concerned with WHY a batch has been withdrawn
A batch can be created to:
- Authorize tickets
- Refund tickets
Copy link
Contributor

@andrewgordstewart andrewgordstewart left a comment

Choose a reason for hiding this comment

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

Except for the vulnerability, this looks great!

contracts/l2.sol Outdated
@@ -189,4 +189,25 @@ contract L2 is SignatureChecker {

batches[honestStartNonce].status = BatchStatus.Refunded;
}

function refund(uint256 index) public {
Copy link
Contributor

Choose a reason for hiding this comment

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

To make sure I understand refund, it:

  • accepts the index of the ticket we want to refund
  • refunds all tickets whose nonce is <= index that have not yet been authorized

?

If so, it probably deserves some documentation to that effect. (And we should put a short statement in the spec.)

Copy link
Contributor Author

@kerzhner kerzhner Dec 16, 2021

Choose a reason for hiding this comment

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

Done in 3068152

contracts/l2.sol Outdated

function refund(uint256 index) public {
require(
block.timestamp > tickets[index].timestamp + maxAuthDelay,
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: this logic makes me think that tickets[index].timestamp would be more clear if it was renamed to tickets[index].createdAt.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I like it: 1b63880

@kerzhner kerzhner merged commit b56d527 into main Dec 16, 2021
@kerzhner kerzhner deleted the refund branch December 16, 2021 17:30
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.

Add refunds to prototype
2 participants