Add swapToRatio endpoint #8

ewilz · 2021-10-08T16:43:23Z

This PR includes some npm package updates to match smart-order-router sdk versions. I'm thinking to push those first in their own PR to separate out those updates from this work.

Todo:

DRY
Separate out sdk package updates into separate PR

prog

justindomingue · 2021-10-14T13:04:35Z

lib/handlers/quote-to-ratio/injector.ts

+
+const SUPPORTED_CHAINS: ChainId[] = [ ChainId.MAINNET, ChainId.RINKEBY ];
+
+const DEFAULT_TOKEN_LIST = 'https://gateway.ipfs.io/ipns/tokens.uniswap.org';


assuming it's the same list, this could be import DEFAULT_TOKEN_LIST from '@uniswap/default-token-list'; so it's more future-proof

Fine with this since the list is never really updated

justindomingue · 2021-10-14T13:04:54Z

lib/handlers/quote-to-ratio/injector.ts

+import { StaticGasPriceProvider } from '../router-entities/static-gas-price-provider';
+import { QuoteToRatioQueryParams } from './schema/quote-to-ratio-schema';
+
+const SUPPORTED_CHAINS: ChainId[] = [ ChainId.MAINNET, ChainId.RINKEBY ];


is this intentially supporting a different set of chains than the quoter?

so a lot of my PR right now is copy-paste, to get the endpoint working, then I will DRY all the shared code. I'm a bit fire-hosed right now, but a lot of this is actually @willpote 's code, so he might be able to answer these questions better.

The quote endpoint on the API also only supports mainnet and rinkeby right now. We have a task to do arbitrary network support that will come soon

justindomingue · 2021-10-14T13:05:18Z

lib/handlers/quote-to-ratio/injector.ts

+const DEFAULT_TOKEN_LIST = 'https://gateway.ipfs.io/ipns/tokens.uniswap.org';
+
+export type ContainerDependencies = {
+ provider: ethers.providers.JsonRpcProvider;


does the provider need to be a jsonrpc as opposed to baseprovider?

It does since we are using the EIP1559 gas provider that uses eth_feehistory

justindomingue · 2021-10-14T13:08:23Z

package.json

@@ -76,7 +76,7 @@
 "@types/qs": "^6.9.7",
 "@types/stats-lite": "^2.2.0",
 "@uniswap/sdk-core": "^3.0.1",
- "@uniswap/smart-order-router": "1.44.1",
+ "@uniswap/smart-order-router": "1.46.3",


this should probably be ^1.46.3 so dependencies can be resolved better next time there's a conflict

ewilz · 2021-10-14T13:57:54Z

tsconfig.json

@@ -12,7 +12,7 @@
 "strictNullChecks": true,
 "noImplicitThis": true,
 "alwaysStrict": true,
- "noUnusedLocals": true /* Report errors on unused locals. */,
+ "noUnusedLocals": false /* Report errors on unused locals. */,


TODO: don't merge this

willpote · 2021-10-25T18:36:51Z

lib/handlers/handler.ts

+ log.info('event information', {
+ event
+ })
+


Still need these logs?

willpote · 2021-10-25T18:38:23Z

lib/handlers/quote-to-ratio/injector.ts

+import { StaticGasPriceProvider } from '../router-entities/static-gas-price-provider';
+import { QuoteToRatioQueryParams } from './schema/quote-to-ratio-schema';
+
+const SUPPORTED_CHAINS: ChainId[] = [ ChainId.MAINNET, ChainId.RINKEBY ];


The quote endpoint on the API also only supports mainnet and rinkeby right now. We have a task to do arbitrary network support that will come soon

willpote · 2021-10-25T18:39:18Z

lib/handlers/quote-to-ratio/injector.ts

+const DEFAULT_TOKEN_LIST = 'https://gateway.ipfs.io/ipns/tokens.uniswap.org';
+
+export type ContainerDependencies = {
+ provider: ethers.providers.JsonRpcProvider;


It does since we are using the EIP1559 gas provider that uses eth_feehistory

willpote · 2021-10-25T18:40:01Z

lib/handlers/quote-to-ratio/injector.ts

+
+const SUPPORTED_CHAINS: ChainId[] = [ ChainId.MAINNET, ChainId.RINKEBY ];
+
+const DEFAULT_TOKEN_LIST = 'https://gateway.ipfs.io/ipns/tokens.uniswap.org';


Fine with this since the list is never really updated

willpote · 2021-10-25T18:41:36Z

test/integ/quote-to-ratio.test.ts

+ deadline: '360',
+ errorTolerance: 1,
+ maxIterations: 6,


Indenting looks dodgy

jest.config.js

ewilz · 2021-11-02T20:36:39Z

lib/handlers/shared.ts

@@ -0,0 +1,66 @@
+import { AlphaRouterConfig } from '@uniswap/smart-order-router';


mmm open to ideas on where to put the two shared items below

successfully outsourced a few types to a shared injector create injectorSOR get rid of unused dependencies DRYing handlers

jest.config.js

willpote · 2021-11-03T15:44:49Z

lib/handlers/quote-to-ratio/quote-to-ratio.ts

+ token0.wrapped,
+ token1.wrapped,
+ feeAmount
+ ) as unknown as Pool


The casting shouldn't be needed, any idea whats going on?

willpote · 2021-11-03T15:45:55Z

lib/handlers/quote-to-ratio/quote-to-ratio.ts

+ log.error(
+ `Could not find pool.`
+ );
+ return { statusCode: 400, errorCode: 'Could not find pool.' };


Error code should be something like POOL_NOT_FOUND. You can use detail field for 'Could not find pool'

willpote · 2021-11-03T15:47:21Z

lib/handlers/quote-to-ratio/quote-to-ratio.ts

+ pool,
+ tickLower,
+ tickUpper,
+ liquidity: 1,


Does setting liquidity to 1 effect anything?

In any case, feels like we should probably use: https://github.com/Uniswap/v3-sdk/blob/main/src/entities/position.ts#L312-L392

to get the correct liquidity value

hmm...it's weird because liquidity plays absolutely no role in any calculation. we define a position to get the bounds, in many cases this will be a hypothetical position with no liquidity yet (which we would then go to create with the swap and add amounts which we don't know yet.

willpote · 2021-11-03T15:47:57Z

lib/handlers/quote-to-ratio/quote-to-ratio.ts

+ log.info(
+ {
+ token0: token0.symbol,
+ token1: token1.symbol,
+ routingConfig: routingConfig,
+ },
+ `Swap to ratio - token0: ${
+ token0.symbol
+ }, token0Balance: ${
+ token0Balance
+ }, token1: ${
+ token1.symbol
+ }. token1Balance: ${
+ token1Balance
+ }, Chain: ${chainId}`
+ );
+
+ log.info({
+ token0Balance,
+ token1Balance,
+ position,
+ maxIterations,
+ errorTolerance: errorToleranceFraction.toFixed(4),
+ swapParams,
+ routingConfig,
+ }, 'more useful logs')


Could you merge these? Best to minimize number of logs

willpote · 2021-11-03T15:55:45Z

lib/handlers/quote-to-ratio/schema/quote-to-ratio-schema.ts

+ numerator: Joi.string(),
+ denominator: Joi.string(),
+ }).required(),
+ postSwapTargetPool: Joi.any().required(),


Missing validation

willpote · 2021-11-03T15:58:40Z

lib/handlers/quote-to-ratio/injector.ts

+import { QuoteToRatioQueryParams } from './schema/quote-to-ratio-schema';
+import { ContainerInjected, InjectorSOR, RequestInjected } from '../injector-sor';
+
+export class QuoteToRatioHandlerInjector extends InjectorSOR<ISwapToRatio<any, any>, QuoteToRatioQueryParams> {


At this point I think it makes sense to tie down our types to specific implementations i.e. ISwapToRatio<RoutingConfig, SwapAndAddConfig>

This way we can get type checking in this file

willpote · 2021-11-03T15:58:48Z

lib/handlers/quote-to-ratio/quote-to-ratio.ts

+
+export class QuoteToRatioHandler extends APIGLambdaHandler<
+ ContainerInjected,
+ RequestInjected<ISwapToRatio<any, any>>,


Same as above

willpote · 2021-11-03T15:59:09Z

lib/handlers/quote/quote.ts


 export class QuoteHandler extends APIGLambdaHandler<
 ContainerInjected,
- RequestInjected,
+ RequestInjected<IRouter<any>>,


Same as above

willpote · 2021-11-03T16:07:09Z

test/integ/quote-to-ratio.test.ts

+ return new Fraction(JSBI.BigInt(fraction.numerator), JSBI.BigInt(fraction.denominator))
+}
+
+describe('quote-to-ratio', () => {


Can we get some 4xx failure case tests?

Good to test some of the basic validation cases just to check that validation is hooked up (e.g. incorrect address returns 4xx etc.), as well as cases where we don't find a swap? (maybe we can set maxIterations to 0 or 1? or maybe passing in token0Balance = 0 and token1Balance = 0?)

Also what happens if I want to do a range order, but I already have 100% of the token required and 0 of the token not required? Do we still recommend a swap? Do we return 200 or 4xx in that case?

ewilz added 5 commits October 5, 2021 12:02

separate schema out

62f4662

progress on new endpoint

5197fa2

prog

fix jsbi version to eliminate errors

9d06bfb

update smart-order-router

c372dee

Merge remote-tracking branch 'origin/main' into new_endpoint

f7e75c9

justindomingue reviewed Oct 14, 2021

View reviewed changes

ewilz commented Oct 14, 2021

View reviewed changes

ewilz added 5 commits October 14, 2021 14:12

add endpoint and start integration tests

a1c395c

getting 400 error, can't find logs

ed30eab

some logs

8e2b1d7

endpoint deployed, returns 200

d32b157

need to return optimal ratio + pool vars

df86e66

willpote approved these changes Oct 25, 2021

View reviewed changes

ewilz added 4 commits October 27, 2021 17:41

update smart-order-router package

ae278de

add new response parameters

b67a75c

integration tests of different volumes passing

e0ab049

integration tests + some param updates

150e212

ewilz commented Nov 2, 2021

View reviewed changes

jest.config.js Outdated Show resolved Hide resolved

ewilz force-pushed the new_endpoint branch from 3a55ece to b076ace Compare November 2, 2021 20:25

ewilz commented Nov 2, 2021

View reviewed changes

ewilz marked this pull request as ready for review November 2, 2021 20:38

ewilz force-pushed the new_endpoint branch from b076ace to 17e375f Compare November 2, 2021 20:53

DRYing and cleaning

c4b0353

successfully outsourced a few types to a shared injector create injectorSOR get rid of unused dependencies DRYing handlers

ewilz force-pushed the new_endpoint branch from 17e375f to c4b0353 Compare November 2, 2021 20:54

update smart-order-router to nonbroken version ^_^

173850a

willpote reviewed Nov 3, 2021

View reviewed changes

address PR comments

d4a103e

ewilz force-pushed the new_endpoint branch from 18200fb to d4a103e Compare November 3, 2021 16:50

ewilz added 2 commits November 3, 2021 12:53

filling in more types

3e3d4f8

clean up logs

8d594bb

ewilz added 2 commits November 3, 2021 15:35

fix integ tests + add more

1bacbef

more comprehensive tests

14f9a0f

ewilz merged commit 8dec90a into main Nov 3, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add swapToRatio endpoint #8

Add swapToRatio endpoint #8

ewilz commented Oct 8, 2021 •

edited

Loading

justindomingue Oct 14, 2021

willpote Oct 25, 2021

justindomingue Oct 14, 2021

ewilz Oct 14, 2021

willpote Oct 25, 2021

justindomingue Oct 14, 2021

willpote Oct 25, 2021

justindomingue Oct 14, 2021

ewilz Oct 14, 2021

willpote Oct 25, 2021

willpote Oct 25, 2021

willpote Oct 25, 2021

willpote Oct 25, 2021

willpote Oct 25, 2021

ewilz Nov 2, 2021 •

edited

Loading

willpote Nov 3, 2021

willpote Nov 3, 2021

willpote Nov 3, 2021

ewilz Nov 3, 2021 •

edited

Loading

willpote Nov 3, 2021

willpote Nov 3, 2021

willpote Nov 3, 2021

willpote Nov 3, 2021

willpote Nov 3, 2021

willpote Nov 3, 2021


		const SUPPORTED_CHAINS: ChainId[] = [ ChainId.MAINNET, ChainId.RINKEBY ];

		const DEFAULT_TOKEN_LIST = 'https://gateway.ipfs.io/ipns/tokens.uniswap.org';

		@@ -0,0 +1,66 @@
		import { AlphaRouterConfig } from '@uniswap/smart-order-router';

Add swapToRatio endpoint #8

Add swapToRatio endpoint #8

Conversation

ewilz commented Oct 8, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ewilz Nov 2, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ewilz Nov 3, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ewilz commented Oct 8, 2021 •

edited

Loading

ewilz Nov 2, 2021 •

edited

Loading

ewilz Nov 3, 2021 •

edited

Loading