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

Claim as X #6193

Merged
merged 91 commits into from
Dec 10, 2024
Merged

Claim as X #6193

merged 91 commits into from
Dec 10, 2024

Conversation

benisgold
Copy link
Member

@benisgold benisgold commented Oct 11, 2024

Fixes APP-1787

I'm sorry about the commits

Context

Two types of claimables:

Transaction: Are claimed on-chain
Sponsored: Claims still occur on-chain, but can only be initiated via an off-chain request. Rainbow doesn't always receive a tx confirmation from these claims.

This feature lets users swap/bridge their claimable to other tokens/networks. We sandwich (rap) together the claim + approval/swap/bridge transactions by using our internal raps module, which manages the nonce of each tx to ensure that the user does not execute any other txs while still executing the claim rap. For this reason, the feature is only compatible with transaction-type claimables, since we require the nonce from the claim tx to confidently execute the following transactions. If you see dropdown menus to select tokens/networks to swap to on the claim panel, you are looking at a transaction-type claimable.

High Level Claim Flow

  1. Check that claimStatus = "notReady"
  2. If swap/bridge is necessary, get quote and calculate display values for token amount + native currency value that user will receive.
  3. Estimate gas (for claim + approval + swap/bridge txs depending on what is necessary). This step also includes checking user balance to see if they can afford the gas fee + formatting the gas fee for display on the UI.
  4. If gas can be afforded, set claimStatus = "ready". Now the claim button is enabled.

Details on Token/Network Selection

The claimable asset and network should be selected by default. The tokens that appear in the dropdown menu are the claimable asset, ETH, USDC, and the native asset of the currently selected network. All networks are available on the dropdown menu. However, not all tokens/networks are compatible. For example, if the selected token is Degen and the user tries to also select ApeChain as the network that wouldn't be possible. To mitigate this, once an item from the first dropdown is selected (whether it is token/network), the options of the 2nd, unused dropdown are automatically constrained by the 1st selection. Each dropdown has a "reset" option that will reset both dropdowns back to the default so they can see all the options again.

Screen recordings / screenshots

https://www.loom.com/share/7e4aafd11dbc47c6b20bde43ba2d412b

Simulator.Screen.Recording.-.iPhone.16.Pro.-.2024-12-04.at.16.08.21.mp4

What to test

Download the TF 1.9.47 (39) and claim your claimables if you have any. Before claiming try a bunch of different swap combos and see if gas estimation works properly / makes sense.

Copy link
Member

@derHowie derHowie left a comment

Choose a reason for hiding this comment

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

tried out both claim types, swap, xchain swap. No issues on those txs. Good stuff!

proof of work: https://www.loom.com/share/e1ef4389ecb446e2803e0fa495c1876d

@@ -735,7 +735,7 @@ export function PointsContent() {
<Box gap={24}>
{rewardsEnabled && (showClaimYourPoints || showMyEarnings) && (
<Box gap={20}>
{showClaimYourPoints && !isReadOnlyWallet && <ClaimCard claim={claimableBalance.display} value={claimablePrice} />}
Copy link
Member

Choose a reason for hiding this comment

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

Was this related or something we were hardcoding during dev?

Copy link
Member Author

Choose a reason for hiding this comment

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

i decided to enable the ClaimCard (eth rewards) for read only wallets so i can also display eth rewards in the claimables section of the wallet screen for read only wallets. since that is the default behavior for claimables in general

@brunobar79
Copy link
Member

Launch in simulator or device for 0d7f1ab

throw error;
}

const receipt = await tx?.wait();
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
const receipt = await tx?.wait();
const receipt = await tx.wait();

already checking above

transaction,
});

const tx = await wallet?.provider?.getTransaction(result.result.hash);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
const tx = await wallet?.provider?.getTransaction(result.result.hash);
const tx = await wallet.provider?.getTransaction(result.result.hash);

should be defined based on args types

Copy link
Contributor

@walmat walmat left a comment

Choose a reason for hiding this comment

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

Looks good aside from the throw that I mentioned earlier @benisgold

@benisgold benisgold requested a review from walmat December 6, 2024 22:45
@brunobar79
Copy link
Member

Launch in simulator or device for 690c431

@BrodyHughes
Copy link
Member

a couple small q's to ben via DM about expected behavior, but QA looks mostly good. Will approve once we talk it out.

@brunobar79
Copy link
Member

Launch in simulator or device for 12c0de8

@brunobar79
Copy link
Member

Launch in simulator or device for 79df6d0

@brunobar79
Copy link
Member

Launch in simulator or device for 44f76b0

@brunobar79
Copy link
Member

Launch in simulator or device for 62f969a

@benisgold benisgold force-pushed the @benisgold/crosschain-swaps-rap branch from 48354e8 to 9afef38 Compare December 9, 2024 20:45
const unlock = createNewAction('unlock', {
fromAddress: accountAddress,
assetToUnlock: assetToSell,
chainId,
contractAddress: quote.to as Address,
Copy link
Member Author

Choose a reason for hiding this comment

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

this was the bug

Copy link
Member

@BrodyHughes BrodyHughes left a comment

Choose a reason for hiding this comment

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

LGTM!

  • checked claims on each network with test claimables
  • checked swaps
  • checked sends
  • alternated the above three options
  • did a few real claims

@brunobar79
Copy link
Member

Launch in simulator or device for 9dfe240

@brunobar79
Copy link
Member

Launch in simulator or device for 253d815

@benisgold benisgold merged commit 7b33b8d into develop Dec 10, 2024
8 checks passed
@benisgold benisgold deleted the @benisgold/crosschain-swaps-rap branch December 10, 2024 20:55
@benisgold benisgold restored the @benisgold/crosschain-swaps-rap branch December 17, 2024 23:23
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.

6 participants