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.

Sign up for GitHub

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

Handle governor warnings similarly to NTT queue #2831

Open
wants to merge 1 commit into
base: development
Choose a base branch
from
Open

Handle governor warnings similarly to NTT queue #2831

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
69 changes: 53 additions & 16 deletions wormhole-connect/src/views/v2/Bridge/Routes/SingleRoute.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
import Stack from '@mui/material/Stack';
import WarningIcon from '@mui/icons-material/Report';
import { makeStyles } from 'tss-react/mui';
import { amount, routes } from '@wormhole-foundation/sdk';
import { amount, routes, Chain } from '@wormhole-foundation/sdk';

import config from 'config';
import TokenIcon from 'icons/TokenIcons';
Expand Down Expand Up @@ -262,26 +262,36 @@
);
}

// Scan for any warnings that might indicate a risk of transfer delay
const maxDelay =
quote?.warnings?.reduce((max, warning) => {
if (
warning.type === 'DestinationCapacityWarning' &&
warning.delayDurationSec
) {
return Math.max(max, warning.delayDurationSec);
} else if (warning.type === 'GovernorLimitWarning') {
return Math.max(max, 24 * 60 * 60);
} else {
return max;
}
}, 0) || 0;

if (maxDelay > 0) {
messages.push(
<TransferDelayWarning
chain={destChain}
token={config.tokens[destToken].symbol}
duration={maxDelay}
/>,
);
}

for (const warning of quote?.warnings || []) {
Copy link
Contributor

Choose a reason for hiding this comment

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

remove

if (
warning.type === 'DestinationCapacityWarning' &&
warning.delayDurationSec
) {

Check failure on line 294 in wormhole-connect/src/views/v2/Bridge/Routes/SingleRoute.tsx

View workflow job for this annotation

GitHub Actions / lint 

Empty block statement
const symbol = config.tokens[destToken].symbol;
const duration = formatDuration(warning.delayDurationSec);
messages.push(
<div key={`${warning.type}-${warning.delayDurationSec}`}>
<Divider flexItem sx={{ marginTop: '8px' }} />
<Stack direction="row" alignItems="center">
<WarningIcon htmlColor={theme.palette.warning.main} />
<Stack sx={{ padding: '16px' }}>
<Typography color={theme.palette.warning.main} fontSize={14}>
{`Your transfer to ${destChain} may be delayed due to rate limits set by ${symbol}. If your transfer is delayed, you will need to return after ${duration} to complete the transfer. Please consider this before proceeding.`}
</Typography>
</Stack>
</Stack>
</div>,
);
}
}

Expand Down Expand Up @@ -459,4 +469,31 @@
);
};

const TransferDelayWarning = (props: {
token: string;
chain?: Chain;
duration: number;
}) => {
const theme = useTheme();
const duration = formatDuration(props.duration);

return (
<div key="TransferDelayWarning">
<Divider flexItem sx={{ marginTop: '8px' }} />
<Stack direction="row" alignItems="center">
<WarningIcon htmlColor={theme.palette.warning.main} />
<Stack sx={{ padding: '16px' }}>
<Typography color={theme.palette.warning.main} fontSize={14}>
{`Your transfer ${
props.chain ? `to ${props.chain} ` : ''
}may be delayed due to rate limits set by ${
props.token
}. If your transfer is delayed, you will need to return after ${duration} to complete the transfer. Please consider this before proceeding.`}
Copy link
Contributor

Choose a reason for hiding this comment

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

If this is an AutomaticTokenBridge transfer and it gets delayed by the governor, the transfer will be relayed once the VAA is emitted 24 hours later. The user doesn't need to return to complete it manually.

Copy link

Choose a reason for hiding this comment

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

I think this copy is ok. Might be too complicated/confusing to have different copy for manual and automatic.

Copy link
Contributor

@aadam-10 aadam-10 Oct 15, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I think the text should say "after at most 24 hours", because enough room may free up in the rolling window before 24h (unless the tx is over the big tx limit but I guess we don't need to complicate this iteration)

as for later, I believe Wormholescan is able to supply the ETA when a tx is scheduled to be released so future iterations can provide better UX by telling the user how much they'll need to wait.

Copy link

@Wesleyleung Wesleyleung Oct 17, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

imo having a clear and consistent expectation (24 hours) even if its conservative is better than something that is not. As a user, I would be fine just checking back exactly the next day and seeing it done one time, vs thinking it might be done less than 24 hours and repeatedly checking in at random times throughout the day to be disappointed. If it happens to be done sooner and I check in sooner, great, then expectations underpromised overdelivered.

</Typography>
</Stack>
</Stack>
</div>
);
};

export default SingleRoute;
Loading