feat: dvm v2 monitor bots #4317

Reinis-FRP · 2022-12-16T23:58:27Z

Signed-off-by: Reinis Martinsons reinis@umaproject.org

Motivation

Improve DVMv2 monitoring.

Summary

Adds DVMv2 monitor bots.

Details

Following monitor bot modules have been added to emit logs when:

large amount of UMA requests unstake
large amount of UMA is staked
every time a governance proposal is submitted
every time a price request is deleted
every time an emergency proposal is created
if a vote rolls
large UMA transfers out from governor
outsized UMA mints

Testing

Check a box to describe how you tested these changes and list the steps for reviewers to test.

Ran end-to-end test, running the code as in production
New unit tests created
Existing tests adequate, no new tests required
All existing tests pass
Untested

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

* feat: update dvm2.0 upgrade script Signed-off-by: Pablo Maldonado <pablo@umaproject.org> * feat: update starting index Signed-off-by: Pablo Maldonado <pablo@umaproject.org> * refactor: update comments Signed-off-by: Pablo Maldonado <pablo@umaproject.org> * feat: update starting index and starting id Signed-off-by: Pablo Maldonado <pablo@umaproject.org> * fix: governor starting id Signed-off-by: Pablo Maldonado <pablo@umaproject.org> * fix: add previous voting contracts events Signed-off-by: Pablo Maldonado <pablo@umaproject.org> * fix: add previous voting contracts events docs Signed-off-by: Pablo Maldonado <pablo@umaproject.org> * refator: improve comment Signed-off-by: Pablo Maldonado <pablo@umaproject.org> Signed-off-by: Pablo Maldonado <pablo@umaproject.org> Co-authored-by: Pablo Maldonado <pablo@umaproject.org> Co-authored-by: chrismaree <christopher.maree@gmail.com> Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: chrismaree <christopher.maree@gmail.com> Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

Co-authored-by: chrismaree <christopher.maree@gmail.com> Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

…s-v2 (#4318) Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

* feat: new release Captures MerkleTree util change to match across-protocol/contracts-v2 * Update CHANGELOG.md * Update CHANGELOG.md * lint Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

… and implementation (#4324) Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

Co-authored-by: Pablo Maldonado <pablo@umaproject.org> Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

This reverts commit e674408. Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

daywiss

very cool, nice design, couple small nits

daywiss · 2022-12-28T13:18:56Z

packages/scripts/src/monitor-dvm/MonitorDeletion.ts

+    spamRequestIndices: event.args.spamRequestIndices,
+  }));
+  deletionProposals.forEach((proposal) => {
+    logDeletionProposed(logger, proposal, params.chainId);


small nit but not sure why you have the .map, you could just change it to a .forEach and log with one iteration and less code, alternatively you could chain the map into the for each without needing to store the extra variable deletionProposals

I was not able to reduce lines of code as significant part of this deals with passing proposal parameters to logDeletionProposed.

daywiss · 2022-12-28T13:28:09Z

packages/scripts/src/monitor-dvm/README.md

+- `UNSTAKES_ENABLED` is boolean enabling/disabling monitoring large unstake attempts (disabled by default).
+- `UNSTAKE_THRESHOLD` is UMA amount threshold to use when `UNSTAKES_ENABLED=true` (defaults to `"0"`).
+- `STAKES_ENABLED` is boolean enabling/disabling monitoring large amount of UMA staked (disabled by default).
+- `STAKE_THRESHOLD` is UMA amount threshold to use when `STAKES_ENABLED=true` (defaults to `"0"`).


need to specify units

fixed in 201b24c

daywiss · 2022-12-28T13:28:29Z

packages/scripts/src/monitor-dvm/README.md

+- `POLLING_DELAY` is value in seconds for delay between consecutive runs, defaults to 1 minute. If set to 0 then running in serverless mode will exit after the loop.
+- `STARTING_BLOCK_NUMBER` and `ENDING_BLOCK_NUMBER` defines block range to look for events on L1 network. These are mandatory when `POLLING_DELAY=0`.
+- `UNSTAKES_ENABLED` is boolean enabling/disabling monitoring large unstake attempts (disabled by default).
+- `UNSTAKE_THRESHOLD` is UMA amount threshold to use when `UNSTAKES_ENABLED=true` (defaults to `"0"`).


what units?

fixed in 201b24c

daywiss · 2022-12-28T13:30:09Z

packages/scripts/src/monitor-dvm/common.ts

+  botModes: BotModes;
+}
+
+export const initCommonEnvVars = async (env: NodeJS.ProcessEnv): Promise<MonitoringParams> => {


i really like this pattern, but prefer you would have parsed and validated all values here rather than leaving out UNSTAKE_THRESHOLD and others to be referenced in the functions rather than the monitoring params

fixed in 228982d

daywiss · 2022-12-28T13:38:19Z

packages/scripts/src/monitor-dvm/MonitorMints.ts

+import type { MonitoringParams } from "./common";
+
+export async function monitorMints(logger: typeof Logger, params: MonitoringParams): Promise<void> {
+  const mintsThreshold = utils.parseEther(process.env.MINTS_THRESHOLD || "0");


i would either turn this into a parameter or add it to monitoring params, same with all the other envs

fixed in 228982d

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

mrice32

Looks great! A few minor comments

mrice32 · 2022-12-29T07:22:35Z

packages/scripts/src/monitor-dvm/MonitorLogger.ts

+  const identifiers = proposal.spamRequestIndices
+    .map((range) => (range[0].eq(range[1]) ? range[0].toString() : `${range[0]}-${range[1]}`))
+    .join(", ");
+  logger.warn({


nit: should this be an error log since it's so rare?

mrice32 · 2022-12-29T07:22:56Z

packages/scripts/src/monitor-dvm/MonitorLogger.ts

+  },
+  roundId: BigNumber
+): void => {
+  logger.warn({


nit: similarly, I think this is rare enough to be an error log, no?

mrice32 · 2022-12-29T07:40:29Z

packages/scripts/src/monitor-dvm/common.ts

+  params.startingBlock = params.endingBlock + 1;
+  params.endingBlock = latestBlockNumber;


nit: not a huge deal, but it can sometimes be confusing to modify POD that's passed into a method rather than returning the values to update or returning an updated copy. This is because readers often assume simple structs are considered immutable, especially in a method that's named "wait*" (which sounds like it just waits a certain amount of time based on the inputs!).

agree, in 13108bb I updated waitNextBlockRange method to return new block range and it is updated within params only in index.ts

mrice32 · 2022-12-29T07:43:17Z

packages/scripts/src/monitor-dvm/common.ts

+  const botModes = {
+    unstakesEnabled: env.UNSTAKES_ENABLED === "true",
+    stakesEnabled: env.STAKES_ENABLED === "true",
+    governanceEnabled: env.GOVERNANCE_ENABLED === "true",
+    deletionEnabled: env.DELETION_ENABLED === "true",
+    emergencyEnabled: env.EMERGENCY_ENABLED === "true",
+    rolledEnabled: env.ROLLED_ENABLED === "true",
+    governorTransfersEnabled: env.GOVERNOR_TRANSFERS_ENABLED === "true",
+    mintsEnabled: env.MINTS_ENABLED === "true",
+  };


I could see us wanting to make some of these errors rather than warnings. Have you considered adding a config element to configure the log level? It also might just be obvious which ones should be errors, so configuration may be overkill, but I definitely think some are error-worthy events.

agree, for now I just updated hard-coded log levels in b7cd55c. Now emergency proposal, spam deletion, rolled votes, large governor transfers and outsized UMA mints will error.

mrice32 · 2022-12-29T07:57:28Z

packages/scripts/src/monitor-dvm/README.md

+```
+yarn ts-node packages/scripts/src/monitor-dvm/index.ts
+```


nit: This is just my opinion, but I think we should prefer using tsc + node over ts-node in production:

tsc is the official typescript implementation built by the typescript team. ts-node is a separate application maintained by a different group as far as I can tell.

It ensures that all these files are compiled and checked by tsc in our build process. With ts-node, it's possible that we skip building these files entirely in ci or partially build them. If that were the case, we would not get the advantage of typescript checking the types automatically as a part of our development process.

Performance reasons: see here and here.

IMO, it's totally fine to encourage ts-node for development purposes in the docs, but I think the default should always be tsc + node.

agree, fixed

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

daywiss

lgtm

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

md0x

LGTM

Reinis-FRP changed the title ~~feat: implement dvm v2 unstake monitor~~ feat: dvm v2 monitor bots Dec 17, 2022

Reinis-FRP marked this pull request as ready for review December 27, 2022 22:45

Reinis-FRP requested review from mrice32, nicholaspai, chrismaree and md0x as code owners December 27, 2022 22:45

Reinis-FRP marked this pull request as draft December 27, 2022 22:47

Reinis-FRP marked this pull request as ready for review December 28, 2022 00:12

Reinis-FRP and others added 22 commits December 28, 2022 12:02

feat: implement dvm v2 unstake monitor

52a2a7a

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

feat: implement dvm v2 stake monitor

99ff00a

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

feat: implement dvm v2 proposal monitor

a25b3f3

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

feat: implement dvm v2 deletion proposal monitor

07dd062

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

feat: implement dvm v2 emergency proposal monitor

6a6277a

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

feat: implement dvm v2 rolled votes monitor

93dcc4b

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

fix dvm monitor logger imports

40ea3ae

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

fix: address issue in CI (#4315)

ae84fa6

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

build(deps): bump decode-uri-component from 0.2.0 to 0.2.2 (#4301)

b65ea51

Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: chrismaree <christopher.maree@gmail.com> Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

chore: Bump ethers to 5.7.x in common and sdk (#4214)

c9e5e0f

Co-authored-by: chrismaree <christopher.maree@gmail.com> Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

feat: add new health check runner package (#4311)

dee9a99

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

improve: Add MerkleTree improvements made in across-protocol/contract…

b284e6b

…s-v2 (#4318) Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

feat: new release (#4322)

d22ac68

* feat: new release Captures MerkleTree util change to match across-protocol/contracts-v2 * Update CHANGELOG.md * Update CHANGELOG.md * lint Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

feat: Add mainnet forking test case (#4306)

7f551a3

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

fix[OA-audit-L03]: Address erronous docstrings and comments (#4320)

9dc29c2

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

fix[OA-audit-N04]: Add additional index paramaters (#4323)

a876bd8

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

fix[OA-audit-N05]: Address argument name mismatches between interface…

6cf619d

… and implementation (#4324) Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

fix[OA-audit-N06]: Address Missing event parameter (#4325)

ddfa78e

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

fix[OA-audit-N07]: Fix licencing issues accross the OA (#4326)

c546807

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

fix[OA-audit-N08]: Fix address multicaller payability (#4327)

6950409

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

fix[OA-audit-N09]: Missing check for non-zero address (#4336)

ae14dee

Co-authored-by: Pablo Maldonado <pablo@umaproject.org> Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

Reinis-FRP added 5 commits December 28, 2022 12:02

fix

56e4024

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

fix: rename common file

27d1cbe

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

Revert "fix: rename common file"

6530122

This reverts commit e674408. Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

feat: rename dvm monitor

d5d2534

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

fix: improve block update and add comments

406b345

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

Reinis-FRP force-pushed the reinis-frp/dvm-monitor branch from 548b5e3 to 406b345 Compare December 28, 2022 12:03

Reinis-FRP requested a review from daywiss as a code owner December 28, 2022 12:03

Merge branch 'master' into reinis-frp/dvm-monitor

6ffa6e3

daywiss reviewed Dec 28, 2022

View reviewed changes

Reinis-FRP added 2 commits December 28, 2022 17:56

fix: add token units to readme

201b24c

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

fix: parse threshold parameters from main entry point

228982d

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

mrice32 reviewed Dec 29, 2022

View reviewed changes

Reinis-FRP added 5 commits December 29, 2022 09:26

fix: update block range only in main entry point

13108bb

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

fix: update log levels

b7cd55c

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

fix: typescript error

127a37a

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

feat: use tsc

bf1fb77

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

fix: update readme

ac823ef

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

Reinis-FRP requested a review from daywiss December 29, 2022 12:32

daywiss approved these changes Dec 29, 2022

View reviewed changes

Reinis-FRP added 8 commits January 9, 2023 08:56

Merge branch 'master' into reinis-frp/dvm-monitor

f62148c

Merge branch 'master' into reinis-frp/dvm-monitor

c13cd96

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

fix: monitor bots after dvm refactor

e76b862

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

fix:

d73ae78

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

fix:

b1db2d0

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

fix:

fec901f

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

fix:

bd7bdf5

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

fix:

11005bf

Signed-off-by: Reinis Martinsons <reinis@umaproject.org>

md0x approved these changes Jan 10, 2023

View reviewed changes

Reinis-FRP merged commit 5a6602e into master Jan 10, 2023

Reinis-FRP deleted the reinis-frp/dvm-monitor branch January 10, 2023 21:01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: dvm v2 monitor bots #4317

feat: dvm v2 monitor bots #4317

Reinis-FRP commented Dec 16, 2022 •

edited

Loading

daywiss left a comment

daywiss Dec 28, 2022

Reinis-FRP Dec 28, 2022

daywiss Dec 28, 2022

Reinis-FRP Dec 28, 2022

daywiss Dec 28, 2022

Reinis-FRP Dec 28, 2022

daywiss Dec 28, 2022

Reinis-FRP Dec 28, 2022

daywiss Dec 28, 2022

Reinis-FRP Dec 28, 2022

mrice32 left a comment

mrice32 Dec 29, 2022

mrice32 Dec 29, 2022

mrice32 Dec 29, 2022

Reinis-FRP Dec 29, 2022

mrice32 Dec 29, 2022

Reinis-FRP Dec 29, 2022

mrice32 Dec 29, 2022

Reinis-FRP Dec 29, 2022

daywiss left a comment

md0x left a comment

		params.startingBlock = params.endingBlock + 1;
		params.endingBlock = latestBlockNumber;

feat: dvm v2 monitor bots #4317

feat: dvm v2 monitor bots #4317

Conversation

Reinis-FRP commented Dec 16, 2022 • edited Loading

daywiss left a 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

mrice32 left a 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

daywiss left a comment

Choose a reason for hiding this comment

md0x left a comment

Choose a reason for hiding this comment

Reinis-FRP commented Dec 16, 2022 •

edited

Loading