Skip to content
This repository was archived by the owner on Dec 27, 2022. It is now read-only.

Auction#603

Merged
rhlsthrm merged 25 commits into
connext:auctionfrom
1Hive:auction
May 27, 2021
Merged

Auction#603
rhlsthrm merged 25 commits into
connext:auctionfrom
1Hive:auction

Conversation

@viken33

@viken33 viken33 commented May 4, 2021

Copy link
Copy Markdown
Collaborator

The Problem

Auction MVP
Specs

The Solution

This PR is a check-in send to see if we are on the right track and if changes on different modules/files are according to MVP design. Its on auction branch on https://github.com/1Hive/vector.git

Update:
Added an Auction Stack to vector in order to test multi router / auction flow
Basic functionality working, can be tested from server-node RPC but in order to run an auction:
- All channels must be setup first. This is basically because actual method in fee service from router takes the channels as params. A different method is needed to calculate fees based only on PUBIDs (lookup channels, calculate deployment fees, etc). Hence, if channels are not present router throws an error and doesn't respond to auction message.
- Auction must be in the same chain, otherwise routers throw an error about not being able to calculate swapRates/AMM rates. Not sure why.

Main additions:
Start-auction script to deploy 3 routers
3 different router.config files to have different fee rates
Makerfile changes ( make start-auction )

@viken33 viken33 closed this May 12, 2021
@viken33 viken33 reopened this May 12, 2021
Comment thread modules/engine/src/index.ts Outdated
Comment thread modules/engine/src/index.ts Outdated
Comment thread modules/engine/src/index.ts
Comment thread modules/router/src/listener.ts
Comment thread modules/types/src/engine.ts
Comment thread modules/types/src/messaging.ts
Comment thread modules/types/src/schemas/engine.ts
Comment thread modules/utils/src/messaging.ts

@viken33 viken33 left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

added some comments

@rhlsthrm

Copy link
Copy Markdown

This looks AMAZING @viken33 ! A couple missing things:

Comment thread Makefile
Comment thread modules/engine/src/index.ts Outdated
Comment thread modules/engine/src/index.ts Outdated
Comment thread modules/engine/src/index.ts Outdated
Comment thread modules/engine/src/index.ts
Comment thread modules/router/src/listener.ts
Comment thread modules/router/src/listener.ts
Comment thread modules/router/src/listener.ts
Comment thread modules/server-node/examples/0-config.http Outdated
Comment thread modules/types/src/schemas/engine.ts

@rhlsthrm rhlsthrm left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This looks great, thanks for addressing all the changes!

A couple of final things we need:

  • Unit tests
  • Feature flag. We need to add a config item so that we can merge this into our prod branch and disable the feature until we fully test it.

Comment thread modules/engine/src/testing/index.spec.ts
Comment thread modules/router/src/test/services/messaging.spec.ts
@viken33

viken33 commented May 27, 2021

Copy link
Copy Markdown
Collaborator Author

Added unit tests where I found them to be relevant for auction (rpc method on engine, listener on router).
If there are more tests we should add, please point me to the tests specs files or folders and I'll them 👍

@rhlsthrm rhlsthrm changed the base branch from main to auction May 27, 2021 20:55
);

await delay(1_000);
await messaging.onReceiveAuctionMessage(signer.publicIdentifier, inbox, (runAuction) => {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Doesn't this have to get set before you send the auction message? I'm not sure if this set of code is being hit. You might need to return a promise that resolves within this message, since I believe this is the last thing that should happen.

@rhlsthrm rhlsthrm merged commit 7fc0acc into connext:auction May 27, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants