-
Notifications
You must be signed in to change notification settings - Fork 65
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
Improve TWAP order mapping with activeOrderUid
#1785
Merged
Merged
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
88c9d4e
Mark TWAP orders as `unknown` if getting part orders fails
iamacook abdb021
Add `activeOrderUid`
iamacook dfea594
Adjust status calculation logic
iamacook 3734ad1
Fix lint
iamacook 686505d
Check if active part is final part
iamacook f20a158
Check if final order exists
iamacook File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -131,6 +131,7 @@ describe('TwapOrderMapper', () => { | |
}); | ||
|
||
expect(result).toEqual({ | ||
activeOrderUid: null, | ||
buyAmount: '51576509680023161648', | ||
buyToken: { | ||
address: buyToken.address, | ||
|
@@ -323,6 +324,7 @@ describe('TwapOrderMapper', () => { | |
trusted: buyToken.trusted, | ||
}, | ||
class: 'limit', | ||
activeOrderUid: null, | ||
durationOfPart: { | ||
durationType: 'AUTO', | ||
}, | ||
|
@@ -460,6 +462,7 @@ describe('TwapOrderMapper', () => { | |
trusted: buyToken.trusted, | ||
}, | ||
class: 'limit', | ||
activeOrderUid: null, | ||
durationOfPart: { | ||
durationType: 'AUTO', | ||
}, | ||
|
@@ -494,6 +497,109 @@ describe('TwapOrderMapper', () => { | |
}); | ||
}); | ||
|
||
it('should correctly generate the activeOrderUid', async () => { | ||
jest.setSystemTime(new Date('2024-06-12T14:43:59.000Z')); | ||
|
||
configurationService.set('swaps.maxNumberOfParts', 1); | ||
|
||
// We instantiate in tests to be able to set maxNumberOfParts | ||
const mapper = new TwapOrderMapper( | ||
configurationService, | ||
mockLoggingService, | ||
swapOrderHelper, | ||
mockSwapsRepository, | ||
composableCowDecoder, | ||
gpv2OrderHelper, | ||
twapOrderHelper, | ||
new SwapAppsHelper(configurationService, allowedApps), | ||
); | ||
|
||
/** | ||
* @see https://sepolia.etherscan.io/address/0xfdaFc9d1902f4e0b84f65F49f244b32b31013b74 | ||
*/ | ||
const chainId = '11155111'; | ||
const owner = '0x31eaC7F0141837B266De30f4dc9aF15629Bd5381'; | ||
const data = | ||
'0x0d0d9800000000000000000000000000000000000000000000000000000000000000008000000000000000000000000052ed56da04309aca4c3fecc595298d80c2f16bac000000000000000000000000000000000000000000000000000000000000024000000000000000000000000000000000000000000000000000000000000000010000000000000000000000006cf1e9ca41f7611def408122793c358a3d11e5a500000000000000000000000000000000000000000000000000000019011f294a00000000000000000000000000000000000000000000000000000000000000600000000000000000000000000000000000000000000000000000000000000140000000000000000000000000be72e441bf55620febc26715db68d3494213d8cb000000000000000000000000fff9976782d46cc05630d1f6ebab18b2324d6b1400000000000000000000000031eac7f0141837b266de30f4dc9af15629bd538100000000000000000000000000000000000000000000000b941d039eed310b36000000000000000000000000000000000000000000000000087bbc924df9167e0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000200000000000000000000000000000000000000000000000000000000000007080000000000000000000000000000000000000000000000000000000000000000f7be7261f56698c258bf75f888d68a00c85b22fb21958b9009c719eb88aebda00000000000000000000000000000000000000000000000000000000000000000'; | ||
const executionDate = new Date('2024-06-13T14:44:00.000Z'); | ||
|
||
/** | ||
* Note: the status has been adjusted to 'open' to simulate an active order | ||
* @see https://explorer.cow.fi/sepolia/orders/0xdaabe82f86545c66074b5565962e96758979ae80124aabef05e0585149d30f7931eac7f0141837b266de30f4dc9af15629bd5381666b05af?tab=overview | ||
*/ | ||
const part1 = { | ||
creationDate: '2024-06-13T14:14:02.269522Z', | ||
owner: '0x31eac7f0141837b266de30f4dc9af15629bd5381', | ||
uid: '0xdaabe82f86545c66074b5565962e96758979ae80124aabef05e0585149d30f7931eac7f0141837b266de30f4dc9af15629bd5381666b05af', | ||
availableBalance: null, | ||
executedBuyAmount: '691671781640850856', | ||
executedSellAmount: '213586875483862141750', | ||
executedSellAmountBeforeFees: '213586875483862141750', | ||
executedFeeAmount: '0', | ||
executedSurplusFee: '111111111', | ||
invalidated: false, | ||
status: 'open', | ||
class: 'limit', | ||
settlementContract: '0x9008d19f58aabd9ed0d60971565aa8510560ab41', | ||
fullFeeAmount: '0', | ||
solverFee: '0', | ||
isLiquidityOrder: false, | ||
fullAppData: | ||
'{"appCode":"Safe Wallet Swaps","metadata":{"orderClass":{"orderClass":"twap"},"quote":{"slippageBips":1000},"widget":{"appCode":"CoW Swap-SafeApp","environment":"production"}},"version":"1.1.0"}', | ||
sellToken: '0xbe72e441bf55620febc26715db68d3494213d8cb', | ||
buyToken: '0xfff9976782d46cc05630d1f6ebab18b2324d6b14', | ||
receiver: '0x31eac7f0141837b266de30f4dc9af15629bd5381', | ||
sellAmount: '213586875483862141750', | ||
buyAmount: '611289510998251134', | ||
validTo: 1718289839, | ||
appData: | ||
'0xf7be7261f56698c258bf75f888d68a00c85b22fb21958b9009c719eb88aebda0', | ||
feeAmount: '0', | ||
kind: 'sell', | ||
partiallyFillable: false, | ||
sellTokenBalance: 'erc20', | ||
buyTokenBalance: 'erc20', | ||
signingScheme: 'eip1271', | ||
signature: | ||
'0x5fd7e97ddaee378bd0eb30ddf479272accf91761e697bc00e067a268f95f1d2732ed230bd5a25ba2e97094ad7d83dc28a6572da797d6b3e7fc6663bd93efb789fc17e489000000000000000000000000000000000000000000000000000000000000008000000000000000000000000000000000000000000000000000000000000002200000000000000000000000000000000000000000000000000000000000000180000000000000000000000000be72e441bf55620febc26715db68d3494213d8cb000000000000000000000000fff9976782d46cc05630d1f6ebab18b2324d6b1400000000000000000000000031eac7f0141837b266de30f4dc9af15629bd538100000000000000000000000000000000000000000000000b941d039eed310b36000000000000000000000000000000000000000000000000087bbc924df9167e00000000000000000000000000000000000000000000000000000000666b05aff7be7261f56698c258bf75f888d68a00c85b22fb21958b9009c719eb88aebda00000000000000000000000000000000000000000000000000000000000000000f3b277728b3fee749481eb3e0b3b48980dbbab78658fc419025cb16eee34677500000000000000000000000000000000000000000000000000000000000000005a28e9363bb942b639270062aa6bb295f434bcdfc42c97267bf003f272060dc95a28e9363bb942b639270062aa6bb295f434bcdfc42c97267bf003f272060dc90000000000000000000000000000000000000000000000000000000000000280000000000000000000000000000000000000000000000000000000000000002000000000000000000000000000000000000000000000000000000000000000600000000000000000000000000000000000000000000000000000000000000080000000000000000000000000000000000000000000000000000000000000024000000000000000000000000000000000000000000000000000000000000000000000000000000000000000006cf1e9ca41f7611def408122793c358a3d11e5a500000000000000000000000000000000000000000000000000000019011f294a00000000000000000000000000000000000000000000000000000000000000600000000000000000000000000000000000000000000000000000000000000140000000000000000000000000be72e441bf55620febc26715db68d3494213d8cb000000000000000000000000fff9976782d46cc05630d1f6ebab18b2324d6b1400000000000000000000000031eac7f0141837b266de30f4dc9af15629bd538100000000000000000000000000000000000000000000000b941d039eed310b36000000000000000000000000000000000000000000000000087bbc924df9167e0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000200000000000000000000000000000000000000000000000000000000000007080000000000000000000000000000000000000000000000000000000000000000f7be7261f56698c258bf75f888d68a00c85b22fb21958b9009c719eb88aebda00000000000000000000000000000000000000000000000000000000000000000', | ||
interactions: { pre: [], post: [] }, | ||
} as unknown as Order; | ||
|
||
const buyToken = tokenBuilder() | ||
.with('address', getAddress(part1.buyToken)) | ||
.build(); | ||
const sellToken = tokenBuilder() | ||
.with('address', getAddress(part1.sellToken)) | ||
.build(); | ||
const fullAppData = JSON.parse(fakeJson()); | ||
|
||
mockSwapsRepository.getOrder.mockResolvedValueOnce(part1); | ||
mockTokenRepository.getToken.mockImplementation(async ({ address }) => { | ||
// We only need mock part1 addresses as all parts use the same tokens | ||
switch (address) { | ||
case buyToken.address: { | ||
return Promise.resolve(buyToken); | ||
} | ||
case sellToken.address: { | ||
return Promise.resolve(sellToken); | ||
} | ||
default: { | ||
return Promise.reject(new Error(`Token not found: ${address}`)); | ||
} | ||
} | ||
}); | ||
mockSwapsRepository.getFullAppData.mockResolvedValue({ fullAppData }); | ||
|
||
const result = await mapper.mapTwapOrder(chainId, owner, { | ||
data, | ||
executionDate, | ||
}); | ||
|
||
expect(result.activeOrderUid).toEqual( | ||
'0x557cb31a9dbbd23830c57d9fd3bbfc3694e942c161232b6cf696ba3bd11f9d6631eac7f0141837b266de30f4dc9af15629bd5381666b0cb7', | ||
); | ||
}); | ||
|
||
it('should throw an error if source apps are restricted and no fullAppData is available', async () => { | ||
const now = new Date(); | ||
jest.setSystemTime(now); | ||
|
@@ -680,6 +786,7 @@ describe('TwapOrderMapper', () => { | |
}); | ||
|
||
it('should map the TWAP order if source apps are restricted and a part order fullAppData matches any of the allowed apps', async () => { | ||
configurationService.set('swaps.maxNumberOfParts', 2); | ||
configurationService.set('swaps.restrictApps', true); | ||
|
||
// We instantiate in tests to be able to set maxNumberOfParts | ||
|
@@ -841,6 +948,7 @@ describe('TwapOrderMapper', () => { | |
}); | ||
|
||
expect(result).toEqual({ | ||
activeOrderUid: null, | ||
buyAmount: '51576509680023161648', | ||
buyToken: { | ||
address: buyToken.address, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we add these (or similar) testcases for the Order Status mapping?
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added in f20a158. |
||
|
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 test began failing without this.
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.
It's weird, I can't replicate the failure, maybe it's a flaky test for some reason. Or maybe the value is set on another test as the configuration is not cleaned between the tests if I'm not wrong. Anyway, I'm good with setting this explictly here.