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

Create UI for permit2 nonce invalidation #73

Closed
rndquu opened this issue Jun 14, 2023 · 31 comments · Fixed by #89
Closed

Create UI for permit2 nonce invalidation #73

rndquu opened this issue Jun 14, 2023 · 31 comments · Fixed by #89

Comments

@rndquu
Copy link
Member

rndquu commented Jun 14, 2023

This payout page is responsible for bounty claims (claim example). Bounty claims are generated with the help of the uniswap's permit2 contract. Each issue has a unique bounty claim URL thanks to the permit2 nonce.

Sometimes there are cases when we should invalidate nonces for some claim URLs in case of the bot's error or smth else. That is why we need a friendly UI for nonce invalidation.

What should be done:

  1. Add a new button "invalidate" to the https://pay.ubq.fi page
  2. The "invalidate" button should be visible only if the authorized wallet account equals to the permit signer and the nonce is unclaimed
  3. On "invalidate" button click we should send a tx that invalidates the nonce

On-chain example of nonce invalidation here

@Venoox
Copy link
Contributor

Venoox commented Jun 25, 2023

/assign

@ubiquibot
Copy link

ubiquibot bot commented Jun 25, 2023

Too many assigned issues, you have reached your max of 2

@Venoox
Copy link
Contributor

Venoox commented Jun 25, 2023

@rndquu my two PRs are in review so I'll just start working on this anyway

@ubiquibot
Copy link

ubiquibot bot commented Jun 25, 2023

@Venoox The time limit for this bounty is on Mon, 26 Jun 2023 13:28:16 GMT

@Venoox
Copy link
Contributor

Venoox commented Jun 25, 2023

I think we also need a dropdown menu for selecting the chain?

@0x4007
Copy link
Member

0x4007 commented Jun 25, 2023

I think we also need a dropdown menu for selecting the chain?

We are going to signal the selected chain to the user interface via a query parameter in the URL. If I can find the link to the pull request I will leave it here.

@rndquu
Copy link
Member Author

rndquu commented Jun 26, 2023

I think we also need a dropdown menu for selecting the chain?

We are going to signal the selected chain to the user interface via a query parameter in the URL. If I can find the link to the pull request I will leave it here.

I guess you're taking about this issue but permit URL is not connected with the current issue

@rndquu
Copy link
Member Author

rndquu commented Jun 26, 2023

I think we also need a dropdown menu for selecting the chain?

Perhaps it is better to use currently selected chain in metamask

@0x4007
Copy link
Member

0x4007 commented Jun 26, 2023

but permit URL is not connected with the current issue

I think the optimal user flow is to allow a treasurer to be able to click a permit link (in a GitHub comment) and then invalidate it if needed. In this case, all of the permit details, including the network ID, is available to the treasurer.

This flow is a lot more efficient to implement, and less burdensome for the user to perform the function.

@rndquu @Venoox we should update the specification.

@rndquu
Copy link
Member Author

rndquu commented Jun 26, 2023

but permit URL is not connected with the current issue

I think the optimal user flow is to allow a treasurer to be able to click a permit link (in a GitHub comment) and then invalidate it if needed. In this case, all of the permit details, including the network ID, is available to the treasurer.

This flow is a lot more efficient to implement, and less burdensome for the user to perform the function.

@rndquu @Venoox we should update the specification.

So here we should add a new button "invalidate" which redirects a treasurer to "invalidate nonce" page with prefilled inputs, right?

@0x4007
Copy link
Member

0x4007 commented Jun 26, 2023

Why not just handle it on the same page? All we need to do is detect that the permit treasurer wallet is the same as the connected wallet on the client.

@rndquu
Copy link
Member Author

rndquu commented Jun 27, 2023

Why not just handle it on the same page? All we need to do is detect that the permit treasurer wallet is the same as the connected wallet on the client.

Yes, you're right. So if the connected wallet in the https://pay.ubq.fi/ page is a permit2 signer then show the "invalidate" button. On "invalidate" buttion click we should simply sign and send tx without an additional UI for the https://pay.ubq.fi/invalidate-nonce page.

@Venoox sorry that the requirements are updated, could you implement the proposed solution?

@Venoox
Copy link
Contributor

Venoox commented Jun 27, 2023

@Venoox sorry that the requirements are updated, could you implement the proposed solution?

No problem.
Where should the button be placed?

Also correct me if I'm wrong but we cannot differentiate between claimed nonce and invalidated nonce, right?

@rndquu
Copy link
Member Author

rndquu commented Jun 27, 2023

@Venoox sorry that the requirements are updated, could you implement the proposed solution?

No problem. Where should the button be placed?

Also correct me if I'm wrong but we cannot differentiate between claimed nonce and invalidated nonce, right?

Where should the button be placed?

Anywhere you see fit

Also correct me if I'm wrong but we cannot differentiate between claimed nonce and invalidated nonce, right?

We can differentiate but it requires parsing all transactions from the permit2 signer address (i.e. bot's wallet). So keep it simple.

So if nonce is unclaimed then display the "invalidate" button.

@0x4007
Copy link
Member

0x4007 commented Jun 27, 2023

Would you mind updating the spec in your original comment @rndquu

@rndquu
Copy link
Member Author

rndquu commented Jun 27, 2023

@Venoox pls check the updated description

@ubiquibot
Copy link

ubiquibot bot commented Jul 3, 2023

Do you have any updates @Venoox? If you would like to release the bounty back to the DevPool, please comment /unassign

@ubiquibot
Copy link

ubiquibot bot commented Jul 4, 2023

Releasing the bounty back to dev pool because the allocated duration already ended!

@ubiquibot ubiquibot bot unassigned Venoox Jul 4, 2023
@Venoox
Copy link
Contributor

Venoox commented Jul 4, 2023

/assign

@ubiquibot
Copy link

ubiquibot bot commented Jul 4, 2023

Too many assigned issues, you have reached your max of 2

@Venoox
Copy link
Contributor

Venoox commented Jul 4, 2023

I have it ready, waiting for #76

@rndquu
Copy link
Member Author

rndquu commented Jul 4, 2023

I have it ready, waiting for #76

#76 (comment)

@Venoox
Copy link
Contributor

Venoox commented Jul 5, 2023

/assign

@ubiquibot
Copy link

ubiquibot bot commented Jul 5, 2023

@Venoox The time limit for this bounty is on Thu, 06 Jul 2023 14:58:49 GMT

Your currently set address is:
0x999cc482d3b04dd3dF733411687341906989Ec5B
please use /wallet 0x4FDE...BA18 if you want to update it.

@0x4007
Copy link
Member

0x4007 commented Jul 10, 2023

@0xcodercrane why isn't the payment permit generated here?

@0x4007 0x4007 reopened this Jul 10, 2023
@0x4007 0x4007 closed this as completed Jul 10, 2023
@0x4007
Copy link
Member

0x4007 commented Jul 10, 2023

My fear about handling the payment manually is that I believe this can later be regenerated but let's just try and be careful about it I don't want @Venoox to be waiting on compensation due to our internal issues.

@0x4007
Copy link
Member

0x4007 commented Jul 10, 2023

@rndquu
Copy link
Member Author

rndquu commented Aug 8, 2023

@Venoox

It seems that wrong parameters are passed to the invalidateUnorderedNonces() method

Check this permit

It was invalidated from the pay.ubq.fi UI with this transaction with the following params passed to invalidateUnorderedNonces():

wordPos: 299162356108459628660949853587209879738140303067132510505332953655896804320
mask: 48

The thing is that this permit is still valid but the invalidation transaction had been executed earlier

From what I understand the invalidation transaction params should have been the following ones:

wordPos: 299162356108459628660949853587209879738140303067132510505332953655896804320
mask: 281474976710704

Could you fix it?

P.S. Note to myself: do not forget to invalidate this permit when the current issue is fixed

Update: invalidated this permit via UI from the PR fix

@rndquu rndquu reopened this Aug 8, 2023
@ubiquibot ubiquibot bot unassigned Venoox Aug 8, 2023
@ubiquibot
Copy link

ubiquibot bot commented Aug 8, 2023

@Venoox - Releasing the bounty back to dev pool because the allocated duration already ended!
Last activity time: Wed Jul 05 2023 14:58:47 GMT+0000 (Coordinated Universal Time)

@Venoox
Copy link
Contributor

Venoox commented Aug 9, 2023

I wanted to fix it but I see you already went ahead and fixed it

@rndquu rndquu closed this as completed Aug 9, 2023
@ubiquibot
Copy link

ubiquibot bot commented Aug 9, 2023

Permit generation skipped since this issue didn't qualify as bounty

If you enjoy the DevPool experience, please follow Ubiquity on GitHub and star this repo to show your support. It helps a lot!

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 a pull request may close this issue.

3 participants