Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Add slippage service - suggest 3 layered architecture #48

Merged
merged 2 commits into from
Jul 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@ import {
} from '../../../../../schemas';
import { FastifyPluginAsync } from 'fastify';
import { FromSchema, JSONSchema } from 'json-schema-to-ts';
import { getSlippageService } from '@cowprotocol/services';

// TODO: Add this in a follow up PR
// import { ALL_SUPPORTED_CHAIN_IDS } from '@cowprotocol/cow-sdk';

interface Result {
slippageBps: number;
Expand Down Expand Up @@ -38,7 +42,7 @@ const responseSchema = {
description:
'Slippage tolerance in basis points. One basis point is equivalent to 0.01% (1/100th of a percent)',
type: 'number',
examples: [50, 100, 200],
examples: [50, 100, 200], // [ALL_SUPPORTED_CHAIN_IDS],
Copy link
Contributor

Choose a reason for hiding this comment

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

This comment doesn't match the property.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

its part of the TODO above, I will come back to that, I promise

minimum: 0,
maximum: 10000,
},
Expand All @@ -47,6 +51,8 @@ const responseSchema = {

type RouteSchema = FromSchema<typeof routeSchema>;

const slippageService = getSlippageService();

const root: FastifyPluginAsync = async (fastify): Promise<void> => {
// example: http://localhost:3010/chains/1/markets/0x6b175474e89094c44da98b954eedeac495271d0f-0x2260fac5e5542a773aa44fbcfedf7c193bc2c599/slippageTolerance
fastify.get<{
Expand All @@ -67,7 +73,11 @@ const root: FastifyPluginAsync = async (fastify): Promise<void> => {
fastify.log.info(
`Get default slippage for market ${baseTokenAddress}-${quoteTokenAddress} on chain ${chainId}`
);
reply.send({ slippageBps: 50 });
const slippageBps = slippageService.getSlippageBps(
baseTokenAddress,
quoteTokenAddress
);
reply.send({ slippageBps });
}
);
};
Expand Down
18 changes: 18 additions & 0 deletions libs/repositories/.eslintrc.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
{
"extends": ["../../.eslintrc.json"],
"ignorePatterns": ["!**/*"],
"overrides": [
{
"files": ["*.ts", "*.tsx", "*.js", "*.jsx"],
"rules": {}
},
{
"files": ["*.ts", "*.tsx"],
"rules": {}
},
{
"files": ["*.js", "*.jsx"],
"rules": {}
}
]
}
7 changes: 7 additions & 0 deletions libs/repositories/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
# repositories

This library was generated with [Nx](https://nx.dev).

## Running unit tests

Run `nx test repositories` to execute the unit tests via [Jest](https://jestjs.io).
11 changes: 11 additions & 0 deletions libs/repositories/jest.config.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
/* eslint-disable */
export default {
displayName: 'repositories',
preset: '../../jest.preset.js',
testEnvironment: 'node',
transform: {
'^.+\\.[tj]s$': ['ts-jest', { tsconfig: '<rootDir>/tsconfig.spec.json' }],
},
moduleFileExtensions: ['ts', 'js', 'html'],
coverageDirectory: '../../coverage/libs/repositories',
};
30 changes: 30 additions & 0 deletions libs/repositories/project.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
{
"name": "repositories",
"$schema": "../../node_modules/nx/schemas/project-schema.json",
"sourceRoot": "libs/repositories/src",
"projectType": "library",
"targets": {
"lint": {
"executor": "@nx/linter:eslint",
"outputs": ["{options.outputFile}"],
"options": {
"lintFilePatterns": ["libs/repositories/**/*.ts"]
}
},
"test": {
"executor": "@nx/jest:jest",
"outputs": ["{workspaceRoot}/coverage/{projectRoot}"],
"options": {
"jestConfig": "libs/repositories/jest.config.ts",
"passWithNoTests": true
},
"configurations": {
"ci": {
"ci": true,
"codeCoverage": true
}
}
}
},
"tags": []
}
16 changes: 16 additions & 0 deletions libs/repositories/src/UsdRepository.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
import { get } from 'http';

export interface UsdRepository {
getDailyUsdPrice(tokenAddress: string, date: Date): Promise<number>;
}

export class UsdRepositoryMock implements UsdRepository {
async getDailyUsdPrice(tokenAddress: string, date: Date): Promise<number> {
return 1234;
}
}

// TODO: Remove once we have IoC
export function getUsdRepository(): UsdRepository {
return new UsdRepositoryMock();
}
Empty file added libs/repositories/src/index.ts
Empty file.
16 changes: 16 additions & 0 deletions libs/repositories/tsconfig.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
{
"extends": "../../tsconfig.base.json",
"compilerOptions": {
"module": "commonjs"
},
"files": [],
"include": [],
"references": [
{
"path": "./tsconfig.lib.json"
},
{
"path": "./tsconfig.spec.json"
}
]
}
11 changes: 11 additions & 0 deletions libs/repositories/tsconfig.lib.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"extends": "./tsconfig.json",
"compilerOptions": {
"module": "commonjs",
"outDir": "../../dist/out-tsc",
"declaration": true,
"types": ["node"]
},
"exclude": ["jest.config.ts", "src/**/*.spec.ts", "src/**/*.test.ts"],
"include": ["src/**/*.ts"]
}
14 changes: 14 additions & 0 deletions libs/repositories/tsconfig.spec.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
{
"extends": "./tsconfig.json",
"compilerOptions": {
"outDir": "../../dist/out-tsc",
"module": "commonjs",
"types": ["jest", "node"]
},
"include": [
"jest.config.ts",
"src/**/*.test.ts",
"src/**/*.spec.ts",
"src/**/*.d.ts"
]
}
18 changes: 18 additions & 0 deletions libs/services/.eslintrc.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
{
"extends": ["../../.eslintrc.json"],
"ignorePatterns": ["!**/*"],
"overrides": [
{
"files": ["*.ts", "*.tsx", "*.js", "*.jsx"],
"rules": {}
},
{
"files": ["*.ts", "*.tsx"],
"rules": {}
},
{
"files": ["*.js", "*.jsx"],
"rules": {}
}
]
}
7 changes: 7 additions & 0 deletions libs/services/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
# services

This library was generated with [Nx](https://nx.dev).

## Running unit tests

Run `nx test services` to execute the unit tests via [Jest](https://jestjs.io).
11 changes: 11 additions & 0 deletions libs/services/jest.config.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
/* eslint-disable */
export default {
displayName: 'services',
preset: '../../jest.preset.js',
testEnvironment: 'node',
transform: {
'^.+\\.[tj]s$': ['ts-jest', { tsconfig: '<rootDir>/tsconfig.spec.json' }],
},
moduleFileExtensions: ['ts', 'js', 'html'],
coverageDirectory: '../../coverage/libs/services',
};
30 changes: 30 additions & 0 deletions libs/services/project.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
{
"name": "services",
"$schema": "../../node_modules/nx/schemas/project-schema.json",
"sourceRoot": "libs/services/src",
"projectType": "library",
"targets": {
"lint": {
"executor": "@nx/linter:eslint",
"outputs": ["{options.outputFile}"],
"options": {
"lintFilePatterns": ["libs/services/**/*.ts"]
}
},
"test": {
"executor": "@nx/jest:jest",
"outputs": ["{workspaceRoot}/coverage/{projectRoot}"],
"options": {
"jestConfig": "libs/services/jest.config.ts",
"passWithNoTests": true
},
"configurations": {
"ci": {
"ci": true,
"codeCoverage": true
}
}
}
},
"tags": []
}
9 changes: 9 additions & 0 deletions libs/services/src/SlippageService/SlippageService.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import { getSlippageService } from './SlippageService';

const slippageService = getSlippageService();

describe('SlippageService', () => {
it('should return always 50', () => {
expect(slippageService.getSlippageBps('0x0', '0x0')).toEqual(50);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unit test for the service

});
});
20 changes: 20 additions & 0 deletions libs/services/src/SlippageService/SlippageService.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
/**
* BPS (Basis Points)
*/
export type Bps = number;

export interface SlippageService {
getSlippageBps(quoteTokenAddress: string, baseTokenAddress: string): Bps;
}

// TODO: Find good name for the implementation, as Leandro don't like "Impl" suffix, just don't want to couple it to add Coingecko in its name (as that would couple it to the data-source, and the adding a name to specify the algorithm might complicate the name, as this will use some custom logic based on standard deviation, so for now as we plan to have just one implementation, I want to keep it simple. But happy to get ideas here)
Copy link
Contributor

Choose a reason for hiding this comment

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

;)

Copy link
Contributor

Choose a reason for hiding this comment

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

What about: BasicSlippageService
Or, StandardSlippageService, DefaultSlippageService, NaiveSlippageService or even FixedSlippageService or DumbSlippageService at it doesn't calculate anything (yet) :P

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You like it more as Prefix?

The reason I usually add it a s sufix is to use it as a qualifier. Like, SlippageServiceA and SlippageServiceB rather than ASlippageService and BSlippageService

This way we go from the general "SlippageService" to the specific SlippageServiceStrategyX
Makes it easier to search for it in the EDI in my opinion. You type SlippageService and sorted you will see

  • SlippageServiceStrategyX
  • SlippageServiceStrategyY
  • SlippageServiceStrategyZ

Regarding the actual prefix/suffix, I used Impl cause is short and typically there's just one implementation, and when there's more will be cause one is a Mock or a Proxy or Redis or some other qualifier, but most of the time I don't expect more than one implementation.

I will call it Main for now as a trade off

export class SlippageServiceImpl implements SlippageService {
getSlippageBps(_quoteTokenAddress: string, _baseTokenAddress: string): Bps {
return 50;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Before I get into the actual implementation, I want to setup the 3 layers and the IoC.

This way, I can abstract where do we get the USD estimations from, and we can test the algorithm of calculating the slippage independently of where do we get the USD estimation from (actually, for the UsdRepository test will be mocked)

}
}

// TODO: This is just temporal! I will introduce a IoC framework in a follow up
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Temporal factory method. this will disappear in a follow up

export function getSlippageService(): SlippageService {
return new SlippageServiceImpl();
}
1 change: 1 addition & 0 deletions libs/services/src/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export * from './SlippageService/SlippageService';
16 changes: 16 additions & 0 deletions libs/services/tsconfig.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
{
"extends": "../../tsconfig.base.json",
"compilerOptions": {
"module": "commonjs"
},
"files": [],
"include": [],
"references": [
{
"path": "./tsconfig.lib.json"
},
{
"path": "./tsconfig.spec.json"
}
]
}
11 changes: 11 additions & 0 deletions libs/services/tsconfig.lib.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"extends": "./tsconfig.json",
"compilerOptions": {
"module": "commonjs",
"outDir": "../../dist/out-tsc",
"declaration": true,
"types": ["node"]
},
"exclude": ["jest.config.ts", "src/**/*.spec.ts", "src/**/*.test.ts"],
"include": ["src/**/*.ts"]
}
14 changes: 14 additions & 0 deletions libs/services/tsconfig.spec.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
{
"extends": "./tsconfig.json",
"compilerOptions": {
"outDir": "../../dist/out-tsc",
"module": "commonjs",
"types": ["jest", "node"]
},
"include": [
"jest.config.ts",
"src/**/*.test.ts",
"src/**/*.spec.ts",
"src/**/*.d.ts"
]
}
26 changes: 8 additions & 18 deletions tsconfig.base.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,30 +10,20 @@
"importHelpers": true,
"target": "es2015",
"module": "esnext",
"lib": [
"es2020",
"dom"
],
"lib": ["es2020", "dom"],
"skipLibCheck": true,
"skipDefaultLibCheck": true,
"baseUrl": ".",
"resolveJsonModule": true,
"esModuleInterop": true,
"allowJs": true,
"paths": {
"@cowprotocol/abis": [
"libs/abis/src/index.ts"
],
"@cowprotocol/cms-api": [
"libs/cms-api/src/index.ts"
],
"@cowprotocol/notifications": [
"libs/notifications/src/index.ts"
]
"@cowprotocol/abis": ["libs/abis/src/index.ts"],
"@cowprotocol/cms-api": ["libs/cms-api/src/index.ts"],
"@cowprotocol/notifications": ["libs/notifications/src/index.ts"],
"@cowprotocol/repositories": ["libs/repositories/src/index.ts"],
"@cowprotocol/services": ["libs/services/src/index.ts"]
}
},
"exclude": [
"node_modules",
"tmp"
]
}
"exclude": ["node_modules", "tmp"]
}
2 changes: 1 addition & 1 deletion yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -6602,7 +6602,7 @@ node-fetch@^2.6.12:

node-fetch@^3.3.2:
version "3.3.2"
resolved "https://registry.npmjs.org/node-fetch/-/node-fetch-3.3.2.tgz"
resolved "https://registry.yarnpkg.com/node-fetch/-/node-fetch-3.3.2.tgz#d1e889bacdf733b4ff3b2b243eb7a12866a0b78b"
integrity sha512-dRB78srN/l6gqWulah9SrxeYnxeddIG30+GOqK/9OlLVyLg3HPnr6SqOWTWOXKRwC2eGYCkZ59NNuSgvSrpgOA==
dependencies:
data-uri-to-buffer "^4.0.0"
Expand Down
Loading