Adding ability for router to update environment variables after start…

…up (#129) * Adding ability for router to update environment variables after startup
ethereum-optimism · May 4, 2020 · c7901ff · c7901ff
1 parent e863d2f
commit c7901ff
Show file tree

Hide file tree

Showing 7 changed files with 134 additions and 14 deletions.
diff --git a/Dockerfile b/Dockerfile
@@ -4,6 +4,9 @@ WORKDIR /server
 COPY . /server
 RUN yarn
 
+# Copy live env config updates file to /server so that it may be updated while running.
+COPY ./packages/rollup-full-node/config/env_var_updates.config /server
+
 WORKDIR /server/packages/rollup-full-node
 
 EXPOSE 8545

diff --git a/packages/rollup-full-node/config/env_var_updates.config b/packages/rollup-full-node/config/env_var_updates.config
@@ -0,0 +1,16 @@
+# This file will be read on occasion from the LIVE process and variables and values
+# listed here will be set in the process.env. It is intended to be used to override
+# existing environment variables without restarting the process if they need to be overridden.
+
+# Note: Any lines that start with # will be ignored
+# set the value to $DELETE$ to clear any set value it may have.
+
+#DUMMY_ENV_VAR=somevalue
+
+# REQUEST_LIMIT_PERIOD_MILLIS=
+# MAX_TRANSACTIONS_PER_UNIT_TIME=
+# MAX_NON_TRANSACTION_REQUESTS_PER_UNIT_TIME=
+
+# CONTRACT_DEPLOYER_ADDRESS=
+# COMMA_SEPARATED_TO_ADDRESS_WHITELIST=
+# COMMA_SEPARATED_RATE_LIMIT_WHITELISTED_IPS=
diff --git a/packages/rollup-full-node/src/app/account-rate-limiter.ts b/packages/rollup-full-node/src/app/account-rate-limiter.ts
@@ -77,7 +77,10 @@ export class DefaultAccountRateLimiter implements AccountRateLimiter {
     this.requestingIpsSinceLastPurge.add(sourceIpAddress)
 
     const numRequests = this.ipToRequestCounter.get(sourceIpAddress).increment()
-    if (numRequests > this.maxRequestsPerTimeUnit) {
+    if (
+      this.maxRequestsPerTimeUnit !== undefined &&
+      numRequests > this.maxRequestsPerTimeUnit
+    ) {
       throw new RateLimitError(
         sourceIpAddress,
         numRequests,
@@ -104,7 +107,10 @@ export class DefaultAccountRateLimiter implements AccountRateLimiter {
     this.requestingAddressesSinceLastPurge.add(address)
 
     const numRequests = this.addressToRequestCounter.get(address).increment()
-    if (numRequests > this.maxTransactionsPerTimeUnit) {
+    if (
+      this.maxTransactionsPerTimeUnit !== undefined &&
+      numRequests > this.maxTransactionsPerTimeUnit
+    ) {
       throw new TransactionLimitError(
         address,
         numRequests,
@@ -144,7 +150,7 @@ export class DefaultAccountRateLimiter implements AccountRateLimiter {
   private refreshVariables(): void {
     try {
       const envPeriod = Environment.requestLimitPeriodMillis()
-      if (!!envPeriod && this.requestLimitPeriodInMillis !== envPeriod) {
+      if (this.requestLimitPeriodInMillis !== envPeriod) {
         const prevVal = this.requestLimitPeriodInMillis
         this.requestLimitPeriodInMillis = envPeriod
         this.ipToRequestCounter.clear()
@@ -157,10 +163,7 @@ export class DefaultAccountRateLimiter implements AccountRateLimiter {
       }
 
       const envRequestLimit = Environment.maxNonTransactionRequestsPerUnitTime()
-      if (
-        !!envRequestLimit &&
-        this.maxRequestsPerTimeUnit !== envRequestLimit
-      ) {
+      if (this.maxRequestsPerTimeUnit !== envRequestLimit) {
         const prevVal = this.maxRequestsPerTimeUnit
         this.maxRequestsPerTimeUnit = envRequestLimit
         log.info(

diff --git a/packages/rollup-full-node/src/app/routing-handler.ts b/packages/rollup-full-node/src/app/routing-handler.ts
@@ -173,11 +173,7 @@ export class RoutingHandler implements FullnodeHandler {
   private refreshVariables(): void {
     try {
       const envToWhitelist = Environment.transactionToAddressWhitelist()
-      if (
-        !!envToWhitelist &&
-        envToWhitelist.length &&
-        !areEqual(envToWhitelist.sort(), this.toAddressWhitelist.sort())
-      ) {
+      if (!areEqual(envToWhitelist.sort(), this.toAddressWhitelist.sort())) {
         const prevValue = this.toAddressWhitelist
         this.toAddressWhitelist = envToWhitelist
         log.info(
@@ -189,8 +185,6 @@ export class RoutingHandler implements FullnodeHandler {
 
       const envIpWhitelist = Environment.rateLimitWhitelistIpAddresses()
       if (
-        !!envIpWhitelist &&
-        !!envIpWhitelist.length &&
         !areEqual(envIpWhitelist.sort(), this.rateLimiterWhitelistedIps.sort())
       ) {
         const prevValue = this.rateLimiterWhitelistedIps

diff --git a/packages/rollup-full-node/src/exec/fullnode.ts b/packages/rollup-full-node/src/exec/fullnode.ts
@@ -9,6 +9,7 @@ import {
 import {
   ExpressHttpServer,
   getLogger,
+  logError,
   Logger,
   SimpleClient,
 } from '@eth-optimism/core-utils'
@@ -135,6 +136,10 @@ const startRoutingServer = async (): Promise<FullnodeContext> => {
   const baseUrl = `http://${Environment.l2RpcServerHost()}:${Environment.l2RpcServerPort()}`
   log.info(`Listening at ${baseUrl}`)
 
+  setInterval(() => {
+    updateEnvironmentVariables('/server/env_var_updates.config')
+  }, 179_000)
+
   return {
     fullnodeHandler: undefined,
     fullnodeRpcServer,
@@ -341,6 +346,72 @@ const getL1ToL2TransactionProcessor = async (
   return l1ToL2TransactionProcessor
 }
 
+/**
+ * Updates process environment variables from provided update file
+ * if any variables are updated.
+ *
+ * @param updateFilePath The path to the file from which to read env var updates.
+ */
+const updateEnvironmentVariables = (updateFilePath: string) => {
+  try {
+    fs.readFile(updateFilePath, 'utf8', (error, data) => {
+      try {
+        let changesExist: boolean = false
+        if (!!error) {
+          logError(
+            log,
+            `Error reading environment variable updates from ${updateFilePath}`,
+            error
+          )
+          return
+        }
+
+        const lines = data.split('\n')
+        for (const rawLine of lines) {
+          if (!rawLine) {
+            continue
+          }
+          const line = rawLine.trim()
+          if (!line || line.startsWith('#')) {
+            continue
+          }
+
+          const varAssignmentSplit = line.split('=')
+          if (varAssignmentSplit.length !== 2) {
+            log.error(
+              `Invalid updated env variable line: ${line}. Expected some_var_name=somevalue`
+            )
+            continue
+          }
+          const key = varAssignmentSplit[0].trim()
+          const value = varAssignmentSplit[1].trim()
+          if (value === '$DELETE$' && !!process.env[key]) {
+            delete process.env[key]
+            log.info(`Updated process.env.${key} to have no value.`)
+            changesExist = true
+          } else if (value !== process.env[key]) {
+            process.env[key] = value
+            log.info(`Updated process.env.${key} to have value ${value}.`)
+            changesExist = true
+          }
+        }
+      } catch (e) {
+        logError(
+          log,
+          `Error updating environment variables from ${updateFilePath}`,
+          e
+        )
+      }
+    })
+  } catch (e) {
+    logError(
+      log,
+      `Error updating environment variables from ${updateFilePath}`,
+      e
+    )
+  }
+}
+
 /**
  * Gets the appropriate db for this node to use based on whether or not this is run in test mode.
  *

diff --git a/packages/rollup-full-node/test/app/account-rate-limiter.spec.ts b/packages/rollup-full-node/test/app/account-rate-limiter.spec.ts
@@ -71,6 +71,17 @@ describe('Account Rate Limiter', () => {
   })
 
   describe('Environment Variable Refresh -- no change', () => {
+    beforeEach(() => {
+      process.env.REQUEST_LIMIT_PERIOD_MILLIS = '1000'
+      process.env.MAX_TRANSACTIONS_PER_UNIT_TIME = '1'
+      process.env.MAX_NON_TRANSACTION_REQUESTS_PER_UNIT_TIME = '1'
+    })
+    afterEach(() => {
+      delete process.env.REQUEST_LIMIT_PERIOD_MILLIS
+      delete process.env.MAX_TRANSACTIONS_PER_UNIT_TIME
+      delete process.env.MAX_NON_TRANSACTION_REQUESTS_PER_UNIT_TIME
+    })
+
     it('post-refresh: does not rate limit transactions if in range', async () => {
       await sleep(2_000)
       // Should not throw
@@ -121,9 +132,13 @@ describe('Account Rate Limiter', () => {
   describe('Environment Variable Refresh -- duration increased', () => {
     beforeEach(() => {
       process.env.REQUEST_LIMIT_PERIOD_MILLIS = '3000'
+      process.env.MAX_TRANSACTIONS_PER_UNIT_TIME = '1'
+      process.env.MAX_NON_TRANSACTION_REQUESTS_PER_UNIT_TIME = '1'
     })
     afterEach(() => {
       delete process.env.REQUEST_LIMIT_PERIOD_MILLIS
+      delete process.env.MAX_TRANSACTIONS_PER_UNIT_TIME
+      delete process.env.MAX_NON_TRANSACTION_REQUESTS_PER_UNIT_TIME
     })
 
     it('post-refresh: does not rate limit transactions if in range', async () => {
@@ -179,10 +194,14 @@ describe('Account Rate Limiter', () => {
 
   describe('Environment Variable Refresh -- tx limit increased', () => {
     beforeEach(() => {
+      process.env.REQUEST_LIMIT_PERIOD_MILLIS = '1000'
       process.env.MAX_TRANSACTIONS_PER_UNIT_TIME = '2'
+      process.env.MAX_NON_TRANSACTION_REQUESTS_PER_UNIT_TIME = '1'
     })
     afterEach(() => {
+      delete process.env.REQUEST_LIMIT_PERIOD_MILLIS
       delete process.env.MAX_TRANSACTIONS_PER_UNIT_TIME
+      delete process.env.MAX_NON_TRANSACTION_REQUESTS_PER_UNIT_TIME
     })
 
     it('post-refresh: does not rate limit transactions if in range', async () => {
@@ -229,9 +248,13 @@ describe('Account Rate Limiter', () => {
 
   describe('Environment Variable Refresh -- request limit increased', () => {
     beforeEach(() => {
+      process.env.REQUEST_LIMIT_PERIOD_MILLIS = '1000'
+      process.env.MAX_TRANSACTIONS_PER_UNIT_TIME = '1'
       process.env.MAX_NON_TRANSACTION_REQUESTS_PER_UNIT_TIME = '2'
     })
     afterEach(() => {
+      delete process.env.REQUEST_LIMIT_PERIOD_MILLIS
+      delete process.env.MAX_TRANSACTIONS_PER_UNIT_TIME
       delete process.env.MAX_NON_TRANSACTION_REQUESTS_PER_UNIT_TIME
     })
 

diff --git a/packages/rollup-full-node/test/app/routing-handler.spec.ts b/packages/rollup-full-node/test/app/routing-handler.spec.ts
@@ -320,9 +320,13 @@ describe('Routing Handler', () => {
       beforeEach(() => {
         deployerWallet = Wallet.createRandom()
         process.env.CONTRACT_DEPLOYER_ADDRESS = add0x(deployerWallet.address)
+        process.env.COMMA_SEPARATED_RATE_LIMIT_WHITELISTED_IPS = '0.0.0.0'
+        process.env.COMMA_SEPARATED_TO_ADDRESS_WHITELIST = whitelistedTo
       })
       afterEach(() => {
         delete process.env.CONTRACT_DEPLOYER_ADDRESS
+        delete process.env.COMMA_SEPARATED_RATE_LIMIT_WHITELISTED_IPS
+        delete process.env.COMMA_SEPARATED_TO_ADDRESS_WHITELIST
       })
 
       it('allows transactions any address from deployer address', async () => {
@@ -363,9 +367,12 @@ describe('Routing Handler', () => {
           toAddress1,
           toAddress2,
         ].join(',')
+
+        process.env.COMMA_SEPARATED_RATE_LIMIT_WHITELISTED_IPS = '0.0.0.0'
       })
       afterEach(() => {
         delete process.env.COMMA_SEPARATED_TO_ADDRESS_WHITELIST
+        delete process.env.COMMA_SEPARATED_RATE_LIMIT_WHITELISTED_IPS
       })
 
       it('allows transactions to whitelisted addresses', async () => {
@@ -402,9 +409,12 @@ describe('Routing Handler', () => {
           whitelistedIpAddress,
           whitelistedIpAddress2,
         ].join(',')
+
+        process.env.COMMA_SEPARATED_TO_ADDRESS_WHITELIST = whitelistedTo
       })
       afterEach(() => {
         delete process.env.COMMA_SEPARATED_RATE_LIMIT_WHITELISTED_IPS
+        delete process.env.COMMA_SEPARATED_TO_ADDRESS_WHITELIST
       })
 
       it('lets transactions through when not limited', async () => {