-
Notifications
You must be signed in to change notification settings - Fork 434
feat: basic limit orders #2649
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
feat: basic limit orders #2649
Conversation
The latest updates on your projects. Learn more about Vercel for GitHub.
|
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.
Pull Request Overview
This PR introduces basic limit order functionality to the swap interface by implementing a new limit order mode that operates alongside the existing market order functionality. It adds the ability for users to create conditional trades that execute when specific price conditions are met.
Key changes include:
- New limit order modal component with price input functionality and expiry settings
- Integration of CoW Protocol's limit order API for order creation and management
- Enhanced market configuration to enable limit order features on specific networks
Reviewed Changes
Copilot reviewed 20 out of 24 changed files in this pull request and generated 8 comments.
Show a summary per file
File | Description |
---|---|
src/hooks/useStaticRate.ts | New hook for fetching static exchange rates between token pairs |
src/hooks/useGetConnectedWalletType.ts | New utility hook to detect smart contract and Safe wallet types |
src/hooks/switch/useCowSwitchRates.ts | New hook for fetching CoW Protocol rates with limit order support |
src/hooks/switch/cowprotocol.rates.ts | Exports getTokenUsdPrice function for external use and updates return type |
src/components/transactions/Switch/SwitchTypeSelector.tsx | New component for toggling between market and limit order modes |
src/components/transactions/Switch/SwitchModal.tsx | Refactored to support both market and limit order types |
src/components/transactions/Switch/SwitchLimitOrdersModalContent.tsx | Main limit order interface with price inputs and order management |
src/components/transactions/Switch/SwitchLimitOrdersActions.tsx | Action handlers for creating and managing limit orders |
src/components/transactions/Switch/PriceInput.tsx | Price input component for setting limit order execution rates |
src/components/transactions/Switch/ExpirySelector.tsx | Component for selecting order expiration timeframes |
src/ui-config/marketsConfig.tsx | Enables limit order feature for mainnet and arbitrum markets |
src/utils/events.ts | Adds tracking events for switch modal interactions |
src/locales/en/messages.po | Adds localization strings for limit order UI elements |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
loading={rateLoading} | ||
rate={rate} | ||
rateUsd={'0'} | ||
switchRate={console.log} |
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.
The switchRate prop is set to console.log which is likely a debugging leftover. This should be replaced with an actual function or removed if not needed.
switchRate={console.log} |
Copilot uses AI. Check for mistakes.
const [expiry, setExpiry] = useState(Expiry['One week']); | ||
|
||
const [inputAmount, setInputAmount] = useState(''); | ||
// const [outputAmount, setOutputAmount] = useState(''); |
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.
Remove commented out code that is not being used.
// const [outputAmount, setOutputAmount] = useState(''); |
Copilot uses AI. Check for mistakes.
const [ | ||
user, | ||
//addTransaction, | ||
currentMarketData, | ||
trackEvent, | ||
] = useRootStore( | ||
useShallow((state) => [ | ||
state.account, | ||
//state.addTransaction, | ||
state.currentMarketData, | ||
state.trackEvent, | ||
]) | ||
); |
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.
Remove commented out code references to addTransaction that are not being used.
Copilot uses AI. Check for mistakes.
// // Track execution state to pause rate updates during actions | ||
// useEffect(() => { | ||
// const isExecuting = mainTxState.loading || approvalTxState.loading || loadingTxns; | ||
|
||
// setIsExecutingActions?.(isExecuting); | ||
// }, [mainTxState.loading, approvalTxState.loading, loadingTxns, setIsExecutingActions]); | ||
|
||
// useEffect(() => { | ||
// let switchGasLimit = 0; | ||
// if (requiresApproval && !approvalTxState.success) { | ||
// switchGasLimit += Number(APPROVAL_GAS_LIMIT); | ||
// if (requiresApprovalReset) { | ||
// switchGasLimit += Number(APPROVAL_GAS_LIMIT); // Reset approval | ||
// } | ||
// } | ||
// if (isNativeToken(inputToken)) { | ||
// switchGasLimit += Number( | ||
// gasLimitRecommendations[ProtocolAction.withdrawAndSwitch].recommended | ||
// ); | ||
// } | ||
// setGasLimit(switchGasLimit.toString()); | ||
// setShowGasStation(requiresApproval || isNativeToken(inputToken)); | ||
// }, [requiresApproval, approvalTxState, setGasLimit, setShowGasStation, requiresApprovalReset]); |
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.
Remove large blocks of commented out code. If this functionality is needed later, it can be retrieved from version control.
Copilot uses AI. Check for mistakes.
inputToken: TokenInfoWithBalance; | ||
outputToken: TokenInfoWithBalance; | ||
outputAmount: string; | ||
//setShowUSDTResetWarning: (showUSDTResetWarning: boolean) => void; |
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.
Remove commented out interface properties and parameter destructuring that are not being used.
Copilot uses AI. Check for mistakes.
inputAmount, | ||
inputToken, | ||
outputToken, | ||
//setShowUSDTResetWarning, |
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.
Remove commented out interface properties and parameter destructuring that are not being used.
Copilot uses AI. Check for mistakes.
//setShowUSDTResetWarning: (showUSDTResetWarning: boolean) => void; | ||
blocked: boolean; | ||
loading?: boolean; | ||
isWrongNetwork: boolean; | ||
chainId: number; | ||
//setShowGasStation: (showGasStation: boolean) => void; | ||
poolReserve?: ComputedReserveData; | ||
targetReserve?: ComputedReserveData; | ||
// setIsExecutingActions?: (isExecuting: boolean) => void; | ||
} | ||
|
||
export const SwitchLimitOrdersActions = ({ | ||
inputAmount, | ||
inputToken, | ||
outputToken, | ||
//setShowUSDTResetWarning, | ||
blocked, | ||
loading, | ||
isWrongNetwork, | ||
chainId, | ||
outputAmount, | ||
}: // setShowGasStation, | ||
// setIsExecutingActions, |
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.
Remove commented out interface properties and parameter destructuring that are not being used.
//setShowUSDTResetWarning: (showUSDTResetWarning: boolean) => void; | |
blocked: boolean; | |
loading?: boolean; | |
isWrongNetwork: boolean; | |
chainId: number; | |
//setShowGasStation: (showGasStation: boolean) => void; | |
poolReserve?: ComputedReserveData; | |
targetReserve?: ComputedReserveData; | |
// setIsExecutingActions?: (isExecuting: boolean) => void; | |
} | |
export const SwitchLimitOrdersActions = ({ | |
inputAmount, | |
inputToken, | |
outputToken, | |
//setShowUSDTResetWarning, | |
blocked, | |
loading, | |
isWrongNetwork, | |
chainId, | |
outputAmount, | |
}: // setShowGasStation, | |
// setIsExecutingActions, | |
blocked: boolean; | |
loading?: boolean; | |
isWrongNetwork: boolean; | |
chainId: number; | |
poolReserve?: ComputedReserveData; | |
targetReserve?: ComputedReserveData; | |
} | |
export const SwitchLimitOrdersActions = ({ | |
inputAmount, | |
inputToken, | |
outputToken, | |
blocked, | |
loading, | |
isWrongNetwork, | |
chainId, | |
outputAmount, |
Copilot uses AI. Check for mistakes.
|
||
#: src/components/transactions/Switch/ExpirySelector.tsx | ||
msgid "{key}" | ||
msgstr "{key}" |
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.
The localization entry '{key}' appears to be a placeholder. This should be replaced with actual descriptive text or the translation logic should be updated to handle dynamic keys properly.
msgstr "{key}" | |
msgstr "Unknown key: {key}" |
Copilot uses AI. Check for mistakes.
const { | ||
data: approvedAmount, | ||
isFetching: fetchingApprovedAmount, | ||
refetch: fetchApprovedAmount, | ||
} = useApprovedAmount({ | ||
chainId, | ||
token: inputToken.address, | ||
spender: COW_PROTOCOL_VAULT_RELAYER_ADDRESS[chainId as SupportedChainId], | ||
}); | ||
|
||
setLoadingTxns(fetchingApprovedAmount); | ||
|
||
let requiresApproval = false; | ||
if (approvedAmount !== undefined) { | ||
requiresApproval = checkRequiresApproval({ | ||
approvedAmount: approvedAmount.toString(), | ||
amount: inputAmount, | ||
signedAmount: '0', | ||
}); | ||
} | ||
|
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.
USDT requires reset in mainnet
const tradingSdk = new TradingSdk({ chainId, signer, appCode: HEADER_WIDGET_APP_CODE }); | ||
const receipt = await tradingSdk.postLimitOrder({ | ||
sellAmount: parseUnits(inputAmount, inputToken.decimals).toString(), | ||
buyAmount: parseUnits(outputAmount, outputToken.decimals).toString(), | ||
kind: OrderKind.SELL, | ||
sellToken: inputToken.address, | ||
buyToken: outputToken.address, | ||
sellTokenDecimals: inputToken.decimals, | ||
buyTokenDecimals: outputToken.decimals, | ||
validFor: 86400, // 24 hours | ||
}); |
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 should re-use helpers in cow herlpers file to avoid missing stuff we learned in past integrations, e.g. partnerfee.
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 dont think there is a function there that can be used, maybe getUnsignedOrder could work with a little rework
buyToken: outputToken.address, | ||
sellTokenDecimals: inputToken.decimals, | ||
buyTokenDecimals: outputToken.decimals, | ||
validFor: 86400, // 24 hours |
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.
this shoul;d come from the picker right?
const receipt = await tradingSdk.postLimitOrder({ | ||
sellAmount: parseUnits(inputAmount, inputToken.decimals).toString(), | ||
buyAmount: parseUnits(outputAmount, outputToken.decimals).toString(), | ||
kind: OrderKind.SELL, |
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.
for limit orders we should be able to support buy orders by changing the buy side right?
Would be nice if we could have the history item show 'Limit Order' instead of 'Swap'
|
const CancelCowOrderModal = dynamic(() => | ||
import('src/components/transactions/CancelCowOrder/CancelCowOrderModal').then( | ||
(module) => module.CancelCowOrderModal | ||
) | ||
); |
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.
thanks for adding
|
📦 Next.js Bundle Analysis for aave-uiThis analysis was generated by the Next.js Bundle Analysis action. 🤖 This PR introduced no changes to the JavaScript bundle! 🙌 |
|
📦 Next.js Bundle Analysis for aave-uiThis analysis was generated by the Next.js Bundle Analysis action. 🤖 This PR introduced no changes to the JavaScript bundle! 🙌 |
|
📦 Next.js Bundle Analysis for aave-uiThis analysis was generated by the Next.js Bundle Analysis action. 🤖 This PR introduced no changes to the JavaScript bundle! 🙌 |
General Changes
Developer Notes
Add any notes here that may be helpful for reviewers.
Reviewer Checklist
Please ensure you, as the reviewer(s), have gone through this checklist to ensure that the code changes are ready to ship safely and to help mitigate any downstream issues that may occur.
.env.example
file as well as the pertinant.github/actions/*
files