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

SHARD-1147 - batch rate limits #91

Merged
merged 6 commits into from
Nov 29, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 58 additions & 0 deletions src/middlewares/rateLimit.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
import { Request, Response, NextFunction } from 'express'
import { RequestersList } from '../utils'
import { CONFIG as config } from '../config'
import { sleep } from '../utils'
import blackList from '../../blacklist.json'
import spammerList from '../../spammerlist.json'

// Export for testing
export const requestersList = new RequestersList(blackList, spammerList)

interface RpcRequest {
method: string
params: any[]
}

async function handleRejection(res: Response, softReject: boolean): Promise<void> {
if (softReject) {
const randomSleepTime = 10 + Math.floor(Math.random() * 10)
await sleep(randomSleepTime * 1000)
res.status(503).send('Network is currently busy. Please try again later.')
} else {
res.status(429).send('Rejected by rate-limiting')
}
}

async function checkRequest(ip: string, request: RpcRequest): Promise<boolean> {
return await requestersList.isRequestOkay(ip, request.method, request.params)
}

export async function rateLimitMiddleware(req: Request, res: Response, next: NextFunction) {
if (!config.rateLimit) {
next()
return
}
let ip = String(req.socket.remoteAddress)
S0naliThakur marked this conversation as resolved.
Show resolved Hide resolved
if (ip.substring(0, 7) == '::ffff:') {
ip = ip.substring(7)
}

const requests: RpcRequest[] = Array.isArray(req.body) ? req.body : [req.body]

try {
const results = await Promise.all(
devendra-shardeum marked this conversation as resolved.
Show resolved Hide resolved
requests.map(request => checkRequest(ip, request))
)

// If any request is not okay, reject the entire batch
if (results.some(result => !result)) {
await handleRejection(res, config.rateLimitOption.softReject)
return
}

next()
} catch (error) {
console.error('Rate limiting error:', error)
res.status(500).send('Internal server error during rate limiting')
S0naliThakur marked this conversation as resolved.
Show resolved Hide resolved
}
}
28 changes: 2 additions & 26 deletions src/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
updateNodeList,
RequestersList,
checkArchiverHealth,
sleep,

Check warning on line 17 in src/server.ts

View workflow job for this annotation

GitHub Actions / ci / QA merge checks

'sleep' is defined but never used
cleanBadNodes,
initSyncTime,
updateEdgeNodeConfig,
Expand All @@ -35,6 +35,7 @@
import { methodWhitelist } from './middlewares/methodWhitelist'
import { isDebugModeMiddlewareLow, rateLimitedDebugAuth } from './middlewares/debugMiddleware'
import { isIPv4 } from 'net'
import { rateLimitMiddleware } from './middlewares/rateLimit'

setDefaultResultOrder('ipv4first')

Expand Down Expand Up @@ -161,7 +162,7 @@
res.send(`counts reset ${Date.now()}`)
})

const requestersList = new RequestersList(blackList, spammerList)

Check warning on line 165 in src/server.ts

View workflow job for this annotation

GitHub Actions / ci / QA merge checks

'requestersList' is assigned a value but never used

interface CustomError extends Error {
status?: number
Expand All @@ -181,32 +182,7 @@
next()
})

app.use(async (req: Request, res: Response, next: NextFunction) => {
if (!config.rateLimit) {
next()
return
}
let ip = String(req.socket.remoteAddress)
if (ip.substring(0, 7) == '::ffff:') {
ip = ip.substring(7)
}
//console.log('IP is ', ip)

const reqParams = req.body.params
const isRequestOkay = await requestersList.isRequestOkay(ip, req.body.method, reqParams)
if (!isRequestOkay) {
if (config.rateLimitOption.softReject) {
const randomSleepTime = 10 + Math.floor(Math.random() * 10)
await sleep(randomSleepTime * 1000)
res.status(503).send('Network is currently busy. Please try again later.')
return
} else {
res.status(503).send('Rejected by rate-limiting')
return
}
}
next()
})
app.use(rateLimitMiddleware)

app.use('/', logRoute)
app.use('/', healthCheckRouter)
Expand Down
82 changes: 82 additions & 0 deletions src/tests/rateLimit.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
import { RequestersList } from '../utils'
import { Request, Response } from 'express'
import { rateLimitMiddleware } from '../middlewares/rateLimit'

describe('Rate Limiting', () => {
let requestersList: RequestersList

Check warning on line 6 in src/tests/rateLimit.test.ts

View workflow job for this annotation

GitHub Actions / ci / QA merge checks

'requestersList' is assigned a value but never used

beforeEach(() => {
requestersList = new RequestersList([], [])
})

it('should allow valid single requests', async () => {
const mockReq = {
socket: { remoteAddress: '127.0.0.1' },
body: {
method: 'eth_call',
params: []
}
} as Request

const mockRes = {
status: jest.fn().mockReturnThis(),
send: jest.fn()
} as unknown as Response

const mockNext = jest.fn()

await rateLimitMiddleware(mockReq, mockRes, mockNext)
expect(mockNext).toHaveBeenCalled()
})

it('should allow valid batch requests', async () => {
const mockReq = {
socket: { remoteAddress: '127.0.0.1' },
body: [
{
method: 'eth_call',
params: []
},
{
method: 'eth_getBalance',
params: []
}
]
} as Request

const mockRes = {
status: jest.fn().mockReturnThis(),
send: jest.fn()
} as unknown as Response

const mockNext = jest.fn()

await rateLimitMiddleware(mockReq, mockRes, mockNext)
expect(mockNext).toHaveBeenCalled()
})

it('should reject when rate limit exceeded', async () => {
const mockReq = {
socket: { remoteAddress: '127.0.0.1' },
body: {
method: 'eth_sendRawTransaction',
params: []
}
} as Request

const mockRes = {
status: jest.fn().mockReturnThis(),
send: jest.fn()
} as unknown as Response

const mockNext = jest.fn()

// Make multiple requests to exceed rate limit
for (let i = 0; i < 61; i++) {
await rateLimitMiddleware(mockReq, mockRes, mockNext)
}

expect(mockRes.status).toHaveBeenCalledWith(503)
expect(mockRes.send).toHaveBeenCalledWith('Rejected by rate-limiting')
})
})
203 changes: 203 additions & 0 deletions test/unit/middlewares/rateLimit.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,203 @@
import { Request, Response } from 'express'
import { rateLimitMiddleware, requestersList } from '../../../src/middlewares/rateLimit'
import { CONFIG } from '../../../src/config'

// Mock the Collector class
jest.mock('../../../src/external/Collector', () => ({
Collector: jest.fn().mockImplementation(() => ({
fetchAccount: jest.fn(),
getBlock: jest.fn(),
getTransactionByHash: jest.fn()
})),
collectorAPI: {
fetchAccount: jest.fn(),
getBlock: jest.fn(),
getTransactionByHash: jest.fn()
}
}))

// Mock the RequestersList instance
jest.mock('../../../src/utils', () => {
return {
RequestersList: jest.fn().mockImplementation(() => ({
isRequestOkay: jest.fn()
})),
sleep: jest.fn().mockImplementation(() => Promise.resolve())
}
})

// Mock config with minimal required properties
jest.mock('../../../src/config', () => ({
CONFIG: {
rateLimit: true,
rateLimitOption: {
softReject: false,
allowedTxCountInCheckInterval: 60
}
}
}))

describe('Rate Limiting', () => {
beforeEach(() => {
jest.clearAllMocks()
// Reset config to default state
CONFIG.rateLimit = true
})

it('should skip rate limiting when disabled in config', async () => {
CONFIG.rateLimit = false

const mockReq = {
socket: { remoteAddress: '127.0.0.1' },
body: {
method: 'eth_call',
params: []
}
} as Request

const mockRes = {
status: jest.fn().mockReturnThis(),
send: jest.fn()
} as unknown as Response

const mockNext = jest.fn()

await rateLimitMiddleware(mockReq, mockRes, mockNext)
expect(mockNext).toHaveBeenCalled()
expect(requestersList.isRequestOkay).not.toHaveBeenCalled()
})

it('should allow valid single requests', async () => {
// Setup the mock for this specific test
jest.spyOn(requestersList, 'isRequestOkay').mockResolvedValueOnce(true)

const mockReq = {
socket: { remoteAddress: '127.0.0.1' },
body: {
method: 'eth_call',
params: []
}
} as Request

const mockRes = {
status: jest.fn().mockReturnThis(),
send: jest.fn()
} as unknown as Response

const mockNext = jest.fn()

await rateLimitMiddleware(mockReq, mockRes, mockNext)
expect(mockNext).toHaveBeenCalled()
expect(requestersList.isRequestOkay).toHaveBeenCalledTimes(1)
})

it('should allow valid batch requests', async () => {
const spy = jest.spyOn(requestersList, 'isRequestOkay')
.mockResolvedValueOnce(true)
.mockResolvedValueOnce(true)

const mockReq = {
socket: { remoteAddress: '127.0.0.1' },
body: [
{
method: 'eth_call',
params: []
},
{
method: 'eth_getBalance',
params: []
}
]
} as Request

const mockRes = {
status: jest.fn().mockReturnThis(),
send: jest.fn()
} as unknown as Response

const mockNext = jest.fn()

await rateLimitMiddleware(mockReq, mockRes, mockNext)
expect(mockNext).toHaveBeenCalled()
expect(spy).toHaveBeenCalledTimes(2)
})

it('should reject when rate limit exceeded', async () => {
jest.spyOn(requestersList, 'isRequestOkay').mockResolvedValue(false)

const mockReq = {
socket: { remoteAddress: '127.0.0.1' },
body: {
method: 'eth_sendRawTransaction',
params: []
}
} as Request

const mockRes = {
status: jest.fn().mockReturnThis(),
send: jest.fn()
} as unknown as Response

const mockNext = jest.fn()

await rateLimitMiddleware(mockReq, mockRes, mockNext)
expect(mockRes.status).toHaveBeenCalledWith(503)
expect(mockRes.send).toHaveBeenCalledWith('Rejected by rate-limiting')
expect(mockNext).not.toHaveBeenCalled()
})

it('should handle soft rejection when configured', async () => {
jest.spyOn(requestersList, 'isRequestOkay').mockResolvedValue(false)

// Temporarily modify config for this test
const originalConfig = CONFIG.rateLimitOption.softReject
CONFIG.rateLimitOption.softReject = true

const mockReq = {
socket: { remoteAddress: '127.0.0.1' },
body: {
method: 'eth_sendRawTransaction',
params: []
}
} as Request

const mockRes = {
status: jest.fn().mockReturnThis(),
send: jest.fn()
} as unknown as Response

const mockNext = jest.fn()

await rateLimitMiddleware(mockReq, mockRes, mockNext)
expect(mockRes.status).toHaveBeenCalledWith(503)
expect(mockRes.send).toHaveBeenCalledWith('Network is currently busy. Please try again later.')
expect(mockNext).not.toHaveBeenCalled()

// Restore original config
CONFIG.rateLimitOption.softReject = originalConfig
})

it('should handle errors gracefully', async () => {
jest.spyOn(requestersList, 'isRequestOkay').mockRejectedValueOnce(new Error('Mock error'))

const mockReq = {
socket: { remoteAddress: '127.0.0.1' },
body: {
method: 'eth_call',
params: []
}
} as Request

const mockRes = {
status: jest.fn().mockReturnThis(),
send: jest.fn()
} as unknown as Response

const mockNext = jest.fn()

await rateLimitMiddleware(mockReq, mockRes, mockNext)
expect(mockRes.status).toHaveBeenCalledWith(500)
expect(mockRes.send).toHaveBeenCalledWith('Internal server error during rate limiting')
expect(mockNext).not.toHaveBeenCalled()
})
})
Loading