Skip to content
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

Deploy to tenderly #35

Merged
merged 18 commits into from
Aug 11, 2023
Merged

Deploy to tenderly #35

merged 18 commits into from
Aug 11, 2023

Conversation

anxolin
Copy link
Contributor

@anxolin anxolin commented Aug 7, 2023

The main motivation of this PR is to deploy to our tenderly account the WatchTower.

However, during the deployment, I had to do some minor changes due to some issues I observed. Sorry for a MISC PR, but just to make it easier I documented in the description the changes and also did some code annotations giving more context (see review mode).

How to deploy this?

Since I didn't know much about this service, I created some documentation that I hope is handy for ONCALL and mantainers of this project.

It shows how to deploy, gives context of what it does. It Explains and gives pointers for all the related concepts you need to know to understand this project, so anyone can get up to speed.

Additionally this document will point out about the evolution (replacement by a distributed version)

https://www.notion.so/cownation/WatchTower-legacy-Tenderly-Composable-CoW-Indexer-8e62049feb6e440d9b097f83e054ac75?pvs=4

Local test for Tenderly Web3 Actions

To debug some issues I had to run Tenderly Actions locally to shorten the dev cycle.

  • I realized there was no instructions in the README, so I added a section
  • The environment vars were not being loaded from .env file, so I added dotenv lib. I think it was working for the CLI but notfor the github actions.

Allow basic Auth in the EthersJS provider

Some RPC nodes require basic auth, so i had to add it.

Allow basic Auth in the EthersJS provider

Now we support to add optionally a user and password for each

NODE_USER_100=your-user
NODE_PASSWORD_100=your-password

Reduce logs

I removed a bunch of logs, so we would write in the log only when there's relevant changes.
For example, we don't write any more "processing block XXX", but we log if we detect a new order, post to the API, etc.

Actually the logs were breaking the indexing (circular JSON dependency), see:
Screenshot at Aug 07 17-19-06

Also, it was making Tenderly to remove all logs and show you nothing:
Screenshot at Aug 07 19-04-40

Improve log messages

The issue above was a bit hard to debug cause i was not sure which log was causing this. This PR in general adds some more context on where things fail, and give you some nice outputs of requests, API responses, etc:

Screenshot at Aug 08 09-36-52

Not included

I still want to reiterate in other PRs to:

  • Receive a notification when there's an error
  • Make the execution fail if there was any errors (currently shows you all GREEN even when there was issues)
  • Handle the duplicate order exception (so its not an error, and we just log that someone else posted the order)
  • [Good to have] Maybe Handle the Settlement Allowance check, because right now we are posting orders that don't have the balance and allowance, so we rely on the backend to tell us. This will save them some resources....

@socket-security
Copy link

New dependencies detected. Learn more about Socket for GitHub ↗︎

Packages Version New capabilities Transitives Size Publisher
dotenv 16.3.1 environment +0 71.6 kB motdotla

.env.example Show resolved Hide resolved
.env.example Show resolved Hide resolved
.env.example Show resolved Hide resolved
actions/package.json Show resolved Hide resolved
actions/register.ts Show resolved Hide resolved
actions/watch.ts Show resolved Hide resolved
actions/watch.ts Show resolved Hide resolved
actions/watch.ts Show resolved Hide resolved
tenderly.yaml Show resolved Hide resolved
tenderly.yaml Show resolved Hide resolved
@anxolin anxolin marked this pull request as ready for review August 8, 2023 10:15
@anxolin anxolin requested review from mfw78 and fleupold August 8, 2023 10:15
mfw78
mfw78 previously approved these changes Aug 8, 2023
Copy link
Contributor

@mfw78 mfw78 left a comment

Choose a reason for hiding this comment

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

Looks great. I'll submit a PR on the env vars to isolate those that are for the forge scripts.

.env.example Show resolved Hide resolved
actions/watch.ts Outdated Show resolved Hide resolved
actions/watch.ts Show resolved Hide resolved
actions/watch.ts Outdated Show resolved Hide resolved
actions/watch.ts Show resolved Hide resolved
actions/watch.ts Outdated Show resolved Hide resolved
actions/watch.ts Show resolved Hide resolved
mfw78
mfw78 previously approved these changes Aug 8, 2023
mfw78
mfw78 previously approved these changes Aug 8, 2023
mfw78
mfw78 previously approved these changes Aug 11, 2023
Copy link
Contributor

@mfw78 mfw78 left a comment

Choose a reason for hiding this comment

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

Looks great, just the minor typo nit.

actions/watch.ts Outdated Show resolved Hide resolved
@anxolin anxolin merged commit 9a64308 into main Aug 11, 2023
2 checks passed
@mfw78 mfw78 deleted the deploy-to-tenderly branch August 11, 2023 12:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants