diff --git a/packages/rollup-full-node/src/app/account-rate-limiter.ts b/packages/rollup-full-node/src/app/account-rate-limiter.ts index 497e670ddb8c6..62fafa3448293 100644 --- a/packages/rollup-full-node/src/app/account-rate-limiter.ts +++ b/packages/rollup-full-node/src/app/account-rate-limiter.ts @@ -1,5 +1,9 @@ /* External imports */ -import { getLogger, TimeBucketedCounter } from '@eth-optimism/core-utils' +import { + getLogger, + logError, + TimeBucketedCounter, +} from '@eth-optimism/core-utils' /* Internal imports */ import { @@ -7,6 +11,7 @@ import { RateLimitError, TransactionLimitError, } from '../types' +import { Environment } from './util' const log = getLogger('routing-handler') @@ -30,10 +35,11 @@ export class DefaultAccountRateLimiter implements AccountRateLimiter { private readonly requestingAddressesSinceLastPurge: Set constructor( - private readonly maxRequestsPerTimeUnit, - private readonly maxTransactionsPerTimeUnit, - private readonly requestLimitPeriodInMillis, - private purgeIntervalMultiplier: number = 1_000 + private maxRequestsPerTimeUnit, + private maxTransactionsPerTimeUnit, + private requestLimitPeriodInMillis, + purgeIntervalMultiplier: number = 1_000, + variableRefreshRateMillis = 300_000 ) { this.requestingIpsSinceLastPurge = new Set() this.requestingAddressesSinceLastPurge = new Set() @@ -48,6 +54,10 @@ export class DefaultAccountRateLimiter implements AccountRateLimiter { setInterval(() => { this.purgeStale(true) }, this.requestLimitPeriodInMillis * (purgeIntervalMultiplier + 5)) + + setInterval(() => { + this.refreshVariables() + }, variableRefreshRateMillis) } /** @@ -94,7 +104,7 @@ export class DefaultAccountRateLimiter implements AccountRateLimiter { this.requestingAddressesSinceLastPurge.add(address) const numRequests = this.addressToRequestCounter.get(address).increment() - if (numRequests > this.maxRequestsPerTimeUnit) { + if (numRequests > this.maxTransactionsPerTimeUnit) { throw new TransactionLimitError( address, numRequests, @@ -127,4 +137,51 @@ export class DefaultAccountRateLimiter implements AccountRateLimiter { }) set.clear() } + + /** + * Refreshes configured member variables from updated Environment Variables. + */ + private refreshVariables(): void { + try { + const envPeriod = Environment.requestLimitPeriodMillis() + if (!!envPeriod && this.requestLimitPeriodInMillis !== envPeriod) { + const prevVal = this.requestLimitPeriodInMillis + this.requestLimitPeriodInMillis = envPeriod + this.ipToRequestCounter.clear() + this.addressToRequestCounter.clear() + this.requestingIpsSinceLastPurge.clear() + this.requestingAddressesSinceLastPurge.clear() + log.info( + `Updated Rate Limit time period from ${prevVal} to ${this.requestLimitPeriodInMillis} millis` + ) + } + + const envRequestLimit = Environment.maxNonTransactionRequestsPerUnitTime() + if ( + !!envRequestLimit && + this.maxRequestsPerTimeUnit !== envRequestLimit + ) { + const prevVal = this.maxRequestsPerTimeUnit + this.maxRequestsPerTimeUnit = envRequestLimit + log.info( + `Updated Max Requests Per unit time value from ${prevVal} to ${this.maxRequestsPerTimeUnit}` + ) + } + + const envTxLimit = Environment.maxTransactionsPerUnitTime() + if (!!envTxLimit && this.maxTransactionsPerTimeUnit !== envTxLimit) { + const prevVal = this.maxTransactionsPerTimeUnit + this.maxTransactionsPerTimeUnit = envTxLimit + log.info( + `Updated Max Transactions Per unit time value from ${prevVal} to ${this.maxTransactionsPerTimeUnit}` + ) + } + } catch (e) { + logError( + log, + `Error updating rate limiter variables from environment variables`, + e + ) + } + } } diff --git a/packages/rollup-full-node/src/app/routing-handler.ts b/packages/rollup-full-node/src/app/routing-handler.ts index 3335247304ddb..4cb91a056a996 100644 --- a/packages/rollup-full-node/src/app/routing-handler.ts +++ b/packages/rollup-full-node/src/app/routing-handler.ts @@ -1,6 +1,7 @@ /* External Imports */ import { Address } from '@eth-optimism/rollup-core' import { + areEqual, getLogger, isJsonRpcErrorResponse, JsonRpcErrorResponse, @@ -22,6 +23,7 @@ import { Web3RpcMethods, web3RpcMethodsExcludingTest, } from '../types' +import { Environment } from './util' const log = getLogger('routing-handler') @@ -47,14 +49,19 @@ export class RoutingHandler implements FullnodeHandler { constructor( private readonly transactionClient: SimpleClient, private readonly readOnlyClient: SimpleClient, - private readonly deployAddress: Address, + private deployAddress: Address, private readonly accountRateLimiter: AccountRateLimiter, - private readonly rateLimiterWhitelistedIps: string[] = [], - private readonly toAddressWhitelist: Address[] = [], + private rateLimiterWhitelistedIps: string[] = [], + private toAddressWhitelist: Address[] = [], private readonly whitelistedMethods: Set = new Set( web3RpcMethodsExcludingTest - ) - ) {} + ), + variableRefreshRateMillis = 300_000 + ) { + setInterval(() => { + this.refreshVariables() + }, variableRefreshRateMillis) + } /** * Handles the provided request by @@ -155,7 +162,59 @@ export class RoutingHandler implements FullnodeHandler { ) { throw new InvalidTransactionDesinationError( tx.to, - this.toAddressWhitelist + Array.from(this.toAddressWhitelist) + ) + } + } + + /** + * Refreshes configured member variables from updated Environment Variables. + */ + private refreshVariables(): void { + try { + const envToWhitelist = Environment.transactionToAddressWhitelist() + if ( + !!envToWhitelist && + envToWhitelist.length && + !areEqual(envToWhitelist.sort(), this.toAddressWhitelist.sort()) + ) { + const prevValue = this.toAddressWhitelist + this.toAddressWhitelist = envToWhitelist + log.info( + `Transaction 'to' address whitelist updated from ${JSON.stringify( + prevValue + )} to ${JSON.stringify(this.toAddressWhitelist)}` + ) + } + + const envIpWhitelist = Environment.rateLimitWhitelistIpAddresses() + if ( + !!envIpWhitelist && + !!envIpWhitelist.length && + !areEqual(envIpWhitelist.sort(), this.rateLimiterWhitelistedIps.sort()) + ) { + const prevValue = this.rateLimiterWhitelistedIps + this.rateLimiterWhitelistedIps = envIpWhitelist + log.info( + `IP whitelist updated from ${JSON.stringify( + prevValue + )} to ${JSON.stringify(this.rateLimiterWhitelistedIps)}` + ) + } + + const deployerAddress = Environment.contractDeployerAddress() + if (!!deployerAddress && deployerAddress !== this.deployAddress) { + const prevValue = this.deployAddress + this.deployAddress = deployerAddress + log.info( + `Deployer address updated from ${prevValue} to ${this.deployAddress}` + ) + } + } catch (e) { + logError( + log, + `Error updating router variables from environment variables`, + e ) } } 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 index af50254404dce..d3892926aa340 100644 --- a/packages/rollup-full-node/test/app/account-rate-limiter.spec.ts +++ b/packages/rollup-full-node/test/app/account-rate-limiter.spec.ts @@ -1,5 +1,5 @@ /* External Imports */ -import { TestUtils } from '@eth-optimism/core-utils' +import { sleep, TestUtils } from '@eth-optimism/core-utils' import { Wallet } from 'ethers' @@ -19,7 +19,13 @@ describe('Account Rate Limiter', () => { let accountRateLimiter: AccountRateLimiter beforeEach(() => { - accountRateLimiter = new DefaultAccountRateLimiter(1, 1, 10_000) + accountRateLimiter = new DefaultAccountRateLimiter( + 1, + 1, + 1_000, + 1_000, + 1_000 + ) }) it('does not rate limit transactions if in range', () => { @@ -34,7 +40,7 @@ describe('Account Rate Limiter', () => { accountRateLimiter.validateRateLimit(ipTwo) }) - it('rate limits transactions if outside of range', () => { + it('rate limits transactions if outside of range', async () => { // Should not throw accountRateLimiter.validateTransactionRateLimit(addressOne) TestUtils.assertThrows(() => { @@ -43,9 +49,13 @@ describe('Account Rate Limiter', () => { // Should not throw accountRateLimiter.validateTransactionRateLimit(addressTwo) + + await sleep(1_050) + // should not throw anymore + accountRateLimiter.validateTransactionRateLimit(addressOne) }) - it('rate limits requests if outside of range', () => { + it('rate limits requests if outside of range', async () => { // Should not throw accountRateLimiter.validateRateLimit(ipOne) TestUtils.assertThrows(() => { @@ -54,5 +64,216 @@ describe('Account Rate Limiter', () => { // Should not throw accountRateLimiter.validateRateLimit(ipTwo) + + await sleep(1_050) + // should not throw anymore + accountRateLimiter.validateRateLimit(ipOne) + }) + + describe('Environment Variable Refresh -- no change', () => { + it('post-refresh: does not rate limit transactions if in range', async () => { + await sleep(2_000) + // Should not throw + accountRateLimiter.validateTransactionRateLimit(addressOne) + accountRateLimiter.validateTransactionRateLimit(addressTwo) + }) + + it('post-refresh: does not rate limit requests if in range', async () => { + await sleep(2_000) + // Should not throw + accountRateLimiter.validateRateLimit(ipOne) + accountRateLimiter.validateRateLimit(ipTwo) + }) + + it('post-refresh: rate limits transactions if outside of range', async () => { + await sleep(2_000) + // Should not throw + accountRateLimiter.validateTransactionRateLimit(addressOne) + TestUtils.assertThrows(() => { + accountRateLimiter.validateTransactionRateLimit(addressOne) + }, TransactionLimitError) + + // Should not throw + accountRateLimiter.validateTransactionRateLimit(addressTwo) + + await sleep(1_050) + // should not throw anymore + accountRateLimiter.validateTransactionRateLimit(addressOne) + }) + + it('post-refresh: rate limits requests if outside of range', async () => { + await sleep(2_000) + // Should not throw + accountRateLimiter.validateRateLimit(ipOne) + TestUtils.assertThrows(() => { + accountRateLimiter.validateRateLimit(ipOne) + }, RateLimitError) + + // Should not throw + accountRateLimiter.validateRateLimit(ipTwo) + + await sleep(1_050) + // should not throw anymore + accountRateLimiter.validateRateLimit(ipOne) + }) + }) + + describe('Environment Variable Refresh -- duration increased', () => { + beforeEach(() => { + process.env.REQUEST_LIMIT_PERIOD_MILLIS = '3000' + }) + afterEach(() => { + delete process.env.REQUEST_LIMIT_PERIOD_MILLIS + }) + + it('post-refresh: does not rate limit transactions if in range', async () => { + await sleep(2_000) + // Should not throw + accountRateLimiter.validateTransactionRateLimit(addressOne) + accountRateLimiter.validateTransactionRateLimit(addressTwo) + }) + + it('post-refresh: does not rate limit requests if in range', async () => { + await sleep(2_000) + // Should not throw + accountRateLimiter.validateRateLimit(ipOne) + accountRateLimiter.validateRateLimit(ipTwo) + }) + + it('post-refresh: rate limits transactions if outside of range', async () => { + await sleep(2_000) + // Should not throw + accountRateLimiter.validateTransactionRateLimit(addressOne) + TestUtils.assertThrows(() => { + accountRateLimiter.validateTransactionRateLimit(addressOne) + }, TransactionLimitError) + + // Should not throw + accountRateLimiter.validateTransactionRateLimit(addressTwo) + + await sleep(1_050) + // should still throw + TestUtils.assertThrows(() => { + accountRateLimiter.validateTransactionRateLimit(addressOne) + }, TransactionLimitError) + }) + + it('post-refresh: rate limits requests if outside of range', async () => { + await sleep(2_000) + // Should not throw + accountRateLimiter.validateRateLimit(ipOne) + TestUtils.assertThrows(() => { + accountRateLimiter.validateRateLimit(ipOne) + }, RateLimitError) + + // Should not throw + accountRateLimiter.validateRateLimit(ipTwo) + + await sleep(1_050) + // should still throw + TestUtils.assertThrows(() => { + accountRateLimiter.validateRateLimit(ipOne) + }, RateLimitError) + }) + }) + + describe('Environment Variable Refresh -- tx limit increased', () => { + beforeEach(() => { + process.env.MAX_TRANSACTIONS_PER_UNIT_TIME = '2' + }) + afterEach(() => { + delete process.env.MAX_TRANSACTIONS_PER_UNIT_TIME + }) + + it('post-refresh: does not rate limit transactions if in range', async () => { + await sleep(2_000) + // Should not throw + accountRateLimiter.validateTransactionRateLimit(addressOne) + accountRateLimiter.validateTransactionRateLimit(addressOne) + accountRateLimiter.validateTransactionRateLimit(addressTwo) + accountRateLimiter.validateTransactionRateLimit(addressTwo) + }) + + it('post-refresh: does not rate limit requests if in range', async () => { + await sleep(2_000) + // Should not throw + accountRateLimiter.validateRateLimit(ipOne) + accountRateLimiter.validateRateLimit(ipTwo) + }) + + it('post-refresh: rate limits transactions if outside of range', async () => { + await sleep(2_000) + // Should not throw + accountRateLimiter.validateTransactionRateLimit(addressOne) + accountRateLimiter.validateTransactionRateLimit(addressOne) + TestUtils.assertThrows(() => { + accountRateLimiter.validateTransactionRateLimit(addressOne) + }, TransactionLimitError) + + // Should not throw + accountRateLimiter.validateTransactionRateLimit(addressTwo) + }) + + it('post-refresh: rate limits requests if outside of range', async () => { + await sleep(2_000) + // Should not throw + accountRateLimiter.validateRateLimit(ipOne) + TestUtils.assertThrows(() => { + accountRateLimiter.validateRateLimit(ipOne) + }, RateLimitError) + + // Should not throw + accountRateLimiter.validateRateLimit(ipTwo) + }) + }) + + describe('Environment Variable Refresh -- request limit increased', () => { + beforeEach(() => { + process.env.MAX_NON_TRANSACTION_REQUESTS_PER_UNIT_TIME = '2' + }) + afterEach(() => { + 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 + accountRateLimiter.validateTransactionRateLimit(addressOne) + accountRateLimiter.validateTransactionRateLimit(addressTwo) + }) + + it('post-refresh: does not rate limit requests if in range', async () => { + await sleep(2_000) + // Should not throw + accountRateLimiter.validateRateLimit(ipOne) + accountRateLimiter.validateRateLimit(ipOne) + accountRateLimiter.validateRateLimit(ipTwo) + accountRateLimiter.validateRateLimit(ipTwo) + }) + + it('post-refresh: rate limits transactions if outside of range', async () => { + await sleep(2_000) + // Should not throw + accountRateLimiter.validateTransactionRateLimit(addressOne) + TestUtils.assertThrows(() => { + accountRateLimiter.validateTransactionRateLimit(addressOne) + }, TransactionLimitError) + + // Should not throw + accountRateLimiter.validateTransactionRateLimit(addressTwo) + }) + + it('post-refresh: rate limits requests if outside of range', async () => { + await sleep(2_000) + // Should not throw + accountRateLimiter.validateRateLimit(ipOne) + accountRateLimiter.validateRateLimit(ipOne) + TestUtils.assertThrows(() => { + accountRateLimiter.validateRateLimit(ipOne) + }, RateLimitError) + + // Should not throw + accountRateLimiter.validateRateLimit(ipTwo) + }) }) }) diff --git a/packages/rollup-full-node/test/app/routing-handler.spec.ts b/packages/rollup-full-node/test/app/routing-handler.spec.ts index 4bdda94621ca4..076591d9511e1 100644 --- a/packages/rollup-full-node/test/app/routing-handler.spec.ts +++ b/packages/rollup-full-node/test/app/routing-handler.spec.ts @@ -1,10 +1,12 @@ /* External Imports */ import { + add0x, JsonRpcError, JsonRpcErrorResponse, JsonRpcResponse, JsonRpcSuccessResponse, SimpleClient, + sleep, TestUtils, } from '@eth-optimism/core-utils' /* Internal Imports */ @@ -67,6 +69,9 @@ const getSignedTransaction = async ( }) } +const whitelistedIpAddress = '9.9.9.9' +const whitelistedIpAddress2 = '8.8.8.8' + const transactionResponse = 'transaction' const readOnlyResponse = 'read only' const transactionResponsePayload: JsonRpcSuccessResponse = { @@ -120,8 +125,6 @@ describe('Routing Handler', () => { let rateLimiter: DummyRateLimiter let routingHandler: RoutingHandler - const whitelistedIpAddress = '9.9.9.9' - beforeEach(() => { rateLimiter = new DummyRateLimiter() routingHandler = new RoutingHandler( @@ -240,7 +243,7 @@ describe('Routing Handler', () => { }, InvalidTransactionDesinationError) }) - it('allows transactions to the whitelisted address from deployer address', async () => { + it('allows transactions to any address from deployer address', async () => { // Should not throw await routingHandler.handleRequest( Web3RpcMethods.sendRawTransaction, @@ -290,6 +293,188 @@ describe('Routing Handler', () => { }) }) + describe('Environment variable refresh', () => { + let routingHandler: RoutingHandler + let rateLimiter: DummyRateLimiter + const whitelistedTo: string = add0x(Wallet.createRandom().address) + + beforeEach(() => { + rateLimiter = new DummyRateLimiter() + routingHandler = new RoutingHandler( + new DummySimpleClient(transactionResponsePayload), + new DummySimpleClient(readOnlyPayload), + '', + rateLimiter, + ['0.0.0.0'], + [whitelistedTo], + new Set([ + Web3RpcMethods.sendRawTransaction, + Web3RpcMethods.networkVersion, + ]), + 1_000 + ) + }) + + describe('Contract deployer address', () => { + let deployerWallet: Wallet + beforeEach(() => { + deployerWallet = Wallet.createRandom() + process.env.CONTRACT_DEPLOYER_ADDRESS = add0x(deployerWallet.address) + }) + afterEach(() => { + delete process.env.CONTRACT_DEPLOYER_ADDRESS + }) + + it('allows transactions any address from deployer address', async () => { + await sleep(1100) + // Should not throw + await routingHandler.handleRequest( + Web3RpcMethods.sendRawTransaction, + [ + await getSignedTransaction( + Wallet.createRandom().address, + deployerWallet + ), + ], + '' + ) + }) + + it('disallows transactions to any address from non-deployer address', async () => { + await sleep(1100) + + await TestUtils.assertThrowsAsync(async () => { + return routingHandler.handleRequest( + Web3RpcMethods.sendRawTransaction, + [await getSignedTransaction()], + '' + ) + }, InvalidTransactionDesinationError) + }) + }) + + describe('To address whitelist', () => { + let toAddress1: string + let toAddress2: string + beforeEach(() => { + toAddress1 = add0x(Wallet.createRandom().address) + toAddress2 = add0x(Wallet.createRandom().address) + process.env.COMMA_SEPARATED_TO_ADDRESS_WHITELIST = [ + toAddress1, + toAddress2, + ].join(',') + }) + afterEach(() => { + delete process.env.COMMA_SEPARATED_TO_ADDRESS_WHITELIST + }) + + it('allows transactions to whitelisted addresses', async () => { + await sleep(1100) + // Should not throw + await routingHandler.handleRequest( + Web3RpcMethods.sendRawTransaction, + [await getSignedTransaction(toAddress1)], + '' + ) + + await routingHandler.handleRequest( + Web3RpcMethods.sendRawTransaction, + [await getSignedTransaction(toAddress2)], + '' + ) + }) + + it('disallows transactions to non-whitelisted address', async () => { + await sleep(1100) + await TestUtils.assertThrowsAsync(async () => { + await routingHandler.handleRequest( + Web3RpcMethods.sendRawTransaction, + [await getSignedTransaction(whitelistedTo)], + '' + ) + }, InvalidTransactionDesinationError) + }) + }) + + describe('Rate limit whitelisted IPs', () => { + beforeEach(() => { + process.env.COMMA_SEPARATED_RATE_LIMIT_WHITELISTED_IPS = [ + whitelistedIpAddress, + whitelistedIpAddress2, + ].join(',') + }) + afterEach(() => { + delete process.env.COMMA_SEPARATED_RATE_LIMIT_WHITELISTED_IPS + }) + + it('lets transactions through when not limited', async () => { + await sleep(1100) + // This should not throw + await routingHandler.handleRequest( + Web3RpcMethods.sendRawTransaction, + [await getSignedTransaction(whitelistedTo)], + '' + ) + }) + + it('does not let transactions through when limited', async () => { + await sleep(1100) + rateLimiter.limitNextTransaction = true + await TestUtils.assertThrowsAsync(async () => { + return routingHandler.handleRequest( + Web3RpcMethods.sendRawTransaction, + [await getSignedTransaction(whitelistedTo)], + '' + ) + }, TransactionLimitError) + }) + + it('lets transactions through when limited and whitelisted', async () => { + await sleep(1100) + rateLimiter.limitNextTransaction = true + // This should not throw + await routingHandler.handleRequest( + Web3RpcMethods.sendRawTransaction, + [await getSignedTransaction(whitelistedTo)], + whitelistedIpAddress + ) + }) + + it('lets requests through when not limited', async () => { + await sleep(1100) + // This should not throw + await routingHandler.handleRequest( + Web3RpcMethods.networkVersion, + [], + '' + ) + }) + + it('does not let requests through when limited', async () => { + await sleep(1100) + rateLimiter.limitNextRequest = true + await TestUtils.assertThrowsAsync(async () => { + return routingHandler.handleRequest( + Web3RpcMethods.networkVersion, + [], + '' + ) + }, RateLimitError) + }) + + it('lets requests through when limited and whitelisted', async () => { + await sleep(1100) + rateLimiter.limitNextRequest = true + // This should not throw + await routingHandler.handleRequest( + Web3RpcMethods.networkVersion, + [], + whitelistedIpAddress + ) + }) + }) + }) + describe('Formatted JSON RPC Responses', () => { let routingHandler: RoutingHandler