-
Notifications
You must be signed in to change notification settings - Fork 169
feat: support intents #446
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
base: main
Are you sure you want to change the base?
Conversation
@fernandomg is attempting to deploy a commit to the Abacus Works Team on Vercel. A member of the Team first needs to authorize it. |
txs[i].orderId ||= options?.orderId; | ||
txs[i].remoteTxHash ||= options?.remoteTxHash; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
{isIntentStandard(t.token.standard) && ( | ||
<> | ||
<span>-</span> | ||
<span> | ||
<i>via intents</i> | ||
</span> | ||
</> | ||
)} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if it's an intent, we let the user know (this is useful in the case that the same token can be bridged via WR or intents)
@@ -86,3 +88,44 @@ export function useEvmWalletBalance( | |||
|
|||
return { balance: data, isError, isLoading }; | |||
} | |||
export async function checkOrderFilled({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
polls for the destination chain Filling
@@ -13,6 +13,7 @@ export enum TransferStatus { | |||
ConfirmingApprove = 'confirming-approve', | |||
SigningTransfer = 'signing-transfer', | |||
ConfirmingTransfer = 'confirming-transfer', | |||
WaitingForFulfillment = 'waiting-for-fulfillment', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a new transfer status
@@ -57,7 +57,7 @@ function filterToIds( | |||
function dedupeTokens(tokens: WarpCoreConfig['tokens']): WarpCoreConfig['tokens'] { | |||
const idToToken: Record<string, WarpCoreConfig['tokens'][number]> = {}; | |||
for (const token of tokens) { | |||
const id = `${token.chainName}|${token.addressOrDenom?.toLowerCase()}`; | |||
const id = `${token.chainName}|${token.addressOrDenom?.toLowerCase()}|${token.standard.toLowerCase()}`; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we need to also differentiate by standard, to permit a token to be birdged via WR and/or intents
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Warning
an example file for testing purposes, this change should be removed before merging.
const ROUTER = '0x6d2175B89315A9EB6c7eA71fDE54Ac0f294aDC34'; | ||
const ITT = '0x5f94BC7Fb4A2779fef010F96b496cD36A909E818'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we have this in the const/config.ts
or const/args.ts
file?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sure, if you want to keep these in the codebase.
My idea was to leave this file untocuched after the PR feedback (i.e.: roll back these changes).
My intention was to share some implementation reference to help on the feedback.
src/features/tokens/balances.ts
Outdated
clearInterval(intervalId); | ||
reject(error); | ||
} | ||
}, 4_000); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we have this in a constant value?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I extracted both values as constants (10 and 4_000) but inside the checkOrderFilled
as it's strictly tied to this solutions.
Would you like me to extract them as constants/env variables?
src/features/tokens/balances.ts
Outdated
const intervalId = setInterval(async () => { | ||
try { | ||
const to = await provider.getBlockNumber(); | ||
const from = to - 10; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what does this 10
represent? Might be a good idea to also a const
value for this
return ( | ||
transfer.remoteTxHash && ( | ||
<TransferProperty | ||
name={name} | ||
value={transfer.remoteTxHash ?? ''} | ||
url={ | ||
multiProvider.tryGetExplorerTxUrl(transfer.destination, { | ||
hash: transfer.remoteTxHash, | ||
}) ?? '' | ||
} | ||
/> | ||
) | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit, can you do something like:
if (!transfer.remoteTxHash) return null
return <TransferProperty ..../>
const orderId = (txReceipt?.receipt as TransactionReceipt)?.logs?.find( | ||
(log) => | ||
log.topics[0].toLowerCase() === | ||
'0x3448bbc2203c608599ad448eeb1007cea04b788ac631f9f558e8dd01a3c27b3d', // `Open` event |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: have this address in a const value
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same as the other constants. The more I create them, the more I feel that they need to be moved into an "intent-related" set of constants.
src/features/tokens/balances.ts
Outdated
@@ -1,5 +1,7 @@ | |||
import { Hyperlane7683__factory as Hyperlane7683Factory } from '@bootnodedev/intents-framework-core'; | |||
import { FilledEvent } from '@bootnodedev/intents-framework-core/dist/src/Base7683'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a way to also import FilledEvent
from '@bootnodedev/intents-framework-core'? without path ...dist/src/Base7683
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed that type import, as it's inferred
This PR may require a history cleanup, but I'm leaving it as testimonial of the multiple iterartions. We went from a proxy (CoreProxy) to only extend WarpCore with the required specific functionalities to support intents, by leveraging EvmAdapters the more we can (check hyperlane-xyz/hyperlane-monorepo#5588)
I'm not sure how HL team does to evaluate SDK/UI modification locally. In my case I used
yalc
(I tried using yarn link, but have multiple issues and couldn't find any guide on how to proceed). As I said, with yalc publish/update I managed to test the SDK changes along with this UI modifications.