Claim vested COW for locked GNO

[jfschwarz]

Summary

This PR adds a section to the profile page for letting users claim vested COW tokens they received for locking GNO.

It's based off #2643, which should be merged first so to make the review of this PR easier.

To Test

1. Open the page Profile

• If the connected account is eligible for vested COW from locked GNO (all addresses listed here: https://dune.xyz/bh2smith/Locked-GNO-Snapshot) it should show a section COW vesting from locked GNO
• The section should show the currently vested amount of tokens
• Users that are not eligible should not see the section

2. Claim the vested COW

• Click the Claim COW button, sign the transaction
• A transaction shall be added to the pending list and the user shall be notified once it completes
• The claim button should be disabled and show Successfully claimed
• The vested amount field should update to show the balance that became vested since clicking the claim button

Background

To prevent a negative impact on page load performance, the merkle proof data for claiming the vested tokens is chunked and will be fetched lazily only when navigating to the profile page. The data can be found in this external repo: https://github.com/gnosis/locked-gno-cow-merkle-distro

[github-actions]

• 🔭 GP Swap: CoW Protocol v2 Swap UI

[elena-zh]

Hey @jfschwarz , great changes!
I'm able to see this section for accounts that have l locked GNO.
However, I'd rather not to make so wide to keep the whole line. Maybe something like this

instead of

Also, I was not able to Claim COW in Gnosis Chain due to this error:

And I was not able to claim in the Mainnet due to I do not have an account (with a PK) that has locked GNO for claiming.

[anxolin]

One quick feedback, which I was mentioning in slack. Do we really need to include these big JSON files?

Ideally we would not include parts of the ABI that are not needed, or generate TS declarations for unused methods.

Also, Im a bit concerned with adding these 2MB files for loading the profile. If you look how we did the airdrop in cowswap, we broke the big fat airdrop file into chunks, and we did a index for them. The index file will be small and will point to the chunk

cowswap/src/custom/state/claim/hooks/index.ts

Line 817 in b970550

return (

[jfschwarz]

However, I'd rather not to make so wide to keep the whole line.

@elena-zh That's a good point! I've now adjusted the flex layout so that only if there's an odd number of boxes, the last one will take the entire width.

As for the claiming, it's currently failing because the vesting contracts have not been funded with COW yet. So we must hold off releasing this PR until this has happened.

@anxolin I like the idea and have split up the merkle proof data into chunks. 👍

[elena-zh]

@jfschwarz , thank you for improving the page layout! Looks much-much better!
But now I see always pending Claim card with this error in the console

The same is for GC:

Could you please fix it?

Thanks

[jfschwarz]

@elena-zh Interesting... 🤔 It seems we found a bug in webpack/babel. The transpilation in the production build was wrong. I've found a way to work around this bug and it's working now.

[elena-zh]

Hey @jfschwarz , great! Now I can send a TX to sign a transaction to the wallet. However, still I'm not able to test changes to the end (run and finish a transaction) due to I do not have personal accounts with locked GNO in any network.

A new tiny issue I have found is related to a mobile/table views: sections content exceeds sections' height, so it would be great to fix this.

Thank you!

[jfschwarz]

Good catch, @elena-zh. fixed now ✔️

[elena-zh]

LGTM now!

[jfschwarz]

The vesting contracts have been funded in the meanwhile, so this would be ready to be merged.

[fairlighteth]

@elena-zh In terms of styling and responsive tests, approved. Thank you @jfschwarz !

[fairlighteth]

Approving based on styles/responsive tests.

[anxolin]

Hi, thanks for splitting the files into chunks. Looks better!

In general in our team we usually refuse to review very big PRs. We believe in smaller PR are easier to review, therefore will get more assurances. This one still has 53K changes. I know most of it are the TS declarations and ABIs, but we usually try to do these ones as independent PRs, and even with these ones out of the way, this PR does many things.

In this case, we can make an exception, although I would ask if we could not include the merkle data in our code. Instead we could load it from an external Gnosis repository. See how we didn't include even our claiming

cowswap/src/custom/state/claim/hooks/index.ts

Line 70 in b970550

export const CLAIMS_REPO = `https://raw.githubusercontent.com/gnosis/cow-merkle-drop/${CLAIMS_REPO_BRANCH}/`

One reason for this, is that this is a one-off thing that makes our git more heavy, and add too many files we won't need in the future.

One additional nitpick, can you add SupportedChainId.MAINNET, etc instead of the hardcoded numbers for the network?

If you make this changes, with the noise out of the way, i can try to review the hooks and logic itself.

BTW, I see you didn't use our logic for finding the chunk given an address and make your own, was it not possible to refactor/reuse?

[jfschwarz]

In this case, we can make an exception, although I would ask if we could not include the merkle data in our code. Instead we could load it from an external Gnosis repository.

Yes, I can move it to an external repo. The disadvantage of that approach is that I will have to manually implement fetch and caching rather than relying on webpack to handle chunk loading automatically, but sure I can do it. I see your point of keeping git light. 👍

BTW, I see you didn't use our logic for finding the chunk given an address and make your own, was it not possible to refactor/reuse?

The existing claim data fetching uses two requests: One fetching the index containing all addresses, a second one for fetching the chunk. Arguably, doing two http roundtrips might turn out to be slower than simply fetching the entire claim data in a single request. Therefore, I used the fact that the claim data is sorted to create an index that is so slim it can be bundled. If you wanna further optimize load performance you might wanna change the existing claim mechanism in the same way.

[W3stside]

I will have to agree with @anxolin here and request changes. There's too much noise coming from this change and I don't believe the potential performance benefit warrants adding this much bulk to the app. Everything else looks great, thanks for the submission. Looking forward to seeing the changes and merging this in.

[elena-zh]

Hey @jfschwarz , I have noticed that:

1. when a user has vested COW from locked GNO
2. and when a user presses on the 'Convert to COW' button to convert vCOW to COW

Then an incorrect 'waiting' modal appears: it says 'Claiming vested COW' instead of the 'Converting vCOW to COW'

See the video: https://watch.screencastify.com/v/zPr5pm8QyM8AqkAabQ4i

So we need to separate these waiting modals:
Convert:

Claim vested:

Thanks!

[jfschwarz]

Another great catch, @elena-zh, thank you! 🙏 I've fixed it

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.
If you think it shouldn't be closed, speak now or forever hold your peace.