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

OG-570 Server readiness stats #666

Merged
merged 12 commits into from
Jul 27, 2021
Merged
Show file tree
Hide file tree
Changes from 10 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
12 changes: 12 additions & 0 deletions packages/common/src/StatsResponse.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
export interface ReadinessInfo {
runningSince: number
currentStateTimestamp: number

totalReadyTime: number
totalNotReadyTime: number
totalReadinessChanges: number
}

export interface StatsResponse extends ReadinessInfo {
totalUptime: number
}
115 changes: 106 additions & 9 deletions packages/dev/test/RelayServer.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,101 @@ contract('RelayServer', function (accounts: Truffle.Accounts) {
})
})

describe('readiness info', function () {
let clock: sinon.SinonFakeTimers
const time = 10000
beforeEach(async function () {
await env.newServerInstanceNoFunding()
await env.fundServer()
await env.relayServer.init()
clock = sinon.useFakeTimers(Date.now())
})
afterEach(function () {
clock.restore()
})

it('should set readiness info in constructor', async function () {
const now = Date.now()
assert.closeTo(env.relayServer.readinessInfo.runningSince, now, 3000)
assert.equal(env.relayServer.readinessInfo.currentStateTimestamp, env.relayServer.readinessInfo.runningSince)
assert.equal(env.relayServer.readinessInfo.totalReadyTime, 0)
assert.equal(env.relayServer.readinessInfo.totalNotReadyTime, 0)

const statsResponse = env.relayServer.statsHandler()
assert.equal(statsResponse.runningSince, env.relayServer.readinessInfo.runningSince)
assert.equal(statsResponse.currentStateTimestamp, env.relayServer.readinessInfo.currentStateTimestamp)
assert.equal(statsResponse.totalUptime, statsResponse.totalReadyTime + statsResponse.totalNotReadyTime)
assert.isTrue(statsResponse.totalUptime > 0)
assert.equal(statsResponse.totalReadinessChanges, 0)
})

it('should keep readiness info when setting to not ready', async function () {
env.relayServer.setReadyState(false)
clock.tick(time)
assert.equal(env.relayServer.readinessInfo.totalReadyTime, 0)
assert.equal(env.relayServer.readinessInfo.currentStateTimestamp - env.relayServer.readinessInfo.runningSince,
env.relayServer.readinessInfo.totalReadyTime + env.relayServer.readinessInfo.totalNotReadyTime)

console.log(env.relayServer.readinessInfo)
const statsResponse = env.relayServer.statsHandler()
assert.equal(statsResponse.runningSince, env.relayServer.readinessInfo.runningSince)
assert.equal(statsResponse.currentStateTimestamp, env.relayServer.readinessInfo.currentStateTimestamp)
assert.equal(statsResponse.totalUptime, statsResponse.totalReadyTime + statsResponse.totalNotReadyTime)
assert.isTrue(statsResponse.totalUptime >= time)
assert.isTrue(statsResponse.totalNotReadyTime >= time)
assert.isTrue(statsResponse.totalReadyTime < time)
assert.equal(statsResponse.totalReadinessChanges, 0)
console.log(statsResponse)
})

it('should keep readiness info when setting to ready', async function () {
env.relayServer.setReadyState(true)
clock.tick(time)
assert.equal(env.relayServer.readinessInfo.currentStateTimestamp - env.relayServer.readinessInfo.runningSince,
env.relayServer.readinessInfo.totalReadyTime + env.relayServer.readinessInfo.totalNotReadyTime)
assert.closeTo(env.relayServer.readinessInfo.totalNotReadyTime, 0, 1000)
// Only one interval, and it's from first uptime until last state change
assert.equal(env.relayServer.readinessInfo.totalReadinessChanges, 1)

console.log(env.relayServer.readinessInfo)
const statsResponse = env.relayServer.statsHandler()
assert.equal(statsResponse.runningSince, env.relayServer.readinessInfo.runningSince)
assert.equal(statsResponse.currentStateTimestamp, env.relayServer.readinessInfo.currentStateTimestamp)
assert.equal(statsResponse.totalUptime, statsResponse.totalReadyTime + statsResponse.totalNotReadyTime)
assert.isTrue(statsResponse.totalUptime >= time)
assert.isTrue(statsResponse.totalReadyTime >= time)
assert.isTrue(statsResponse.totalNotReadyTime < time)
assert.equal(statsResponse.totalReadinessChanges, 1)
console.log(statsResponse)
})

it('should keep readiness info when setting new readiness states', async function () {
env.relayServer.setReadyState(false)
clock.tick(time)
env.relayServer.setReadyState(false)
clock.tick(time)
env.relayServer.setReadyState(true)
clock.tick(time)
env.relayServer.setReadyState(false)
clock.tick(time)
env.relayServer.setReadyState(true)
clock.tick(time)
env.relayServer.setReadyState(false)
assert.equal(env.relayServer.readinessInfo.totalReadyTime, time * 2)
assert.equal(env.relayServer.readinessInfo.totalReadinessChanges, 4)
assert.equal(env.relayServer.readinessInfo.currentStateTimestamp - env.relayServer.readinessInfo.runningSince,
env.relayServer.readinessInfo.totalReadyTime + env.relayServer.readinessInfo.totalNotReadyTime)

console.log(env.relayServer.readinessInfo)
const statsResponse = env.relayServer.statsHandler()
assert.equal(statsResponse.totalReadinessChanges, 4)
assert.isTrue(statsResponse.totalUptime >= 5 * time)
assert.equal(statsResponse.totalReadyTime, time * 2)
assert.isTrue(statsResponse.totalNotReadyTime >= 3 * time)
console.log(statsResponse)
})
})

describe('validation', function () {
const blacklistedPaymaster = '0xdeadfaceffff'
beforeEach(async function () {
Expand Down Expand Up @@ -291,7 +386,8 @@ contract('RelayServer', function (accounts: Truffle.Accounts) {
assert.fail()
} catch (e) {
// eslint-disable-next-line @typescript-eslint/restrict-template-expressions
assert.include(e.message, `network gas price ${env.relayServer.minGasPrice} is higher than max gas price ${env.relayServer.config.maxGasPrice}`)
assert.include(e.message,
`network gas price ${env.relayServer.minGasPrice} is higher than max gas price ${env.relayServer.config.maxGasPrice}`)
} finally {
env.relayServer.config.maxGasPrice = originalMaxPrice
}
Expand Down Expand Up @@ -687,14 +783,15 @@ contract('RelayServer', function (accounts: Truffle.Accounts) {

async function attackTheServer (server: RelayServer): Promise<void> {
const _sendTransactionOrig = server.transactionManager.sendTransaction
server.transactionManager.sendTransaction = async function ({
signer,
method,
destination,
value = '0x',
gasLimit,
gasPrice
}: SendTransactionDetails): Promise<SignedTransactionDetails> {
server.transactionManager.sendTransaction = async function (
{
signer,
method,
destination,
value = '0x',
gasLimit,
gasPrice
}: SendTransactionDetails): Promise<SignedTransactionDetails> {
await rejectingPaymaster.setRevertPreRelayCall(true)
// @ts-ignore
// eslint-disable-next-line @typescript-eslint/return-await
Expand Down
60 changes: 55 additions & 5 deletions packages/dev/test/ServerConfigParams.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,12 @@ import {
import * as fs from 'fs'
import { expectRevert } from '@openzeppelin/test-helpers'
import {
RelayHubInstance,
VersionRegistryInstance
} from '@opengsn/contracts/types/truffle-contracts'
import { string32 } from '@opengsn/common/dist/VersionRegistry'
import { constants } from '@opengsn/common/dist'
import { deployHub } from './TestUtils'

require('source-map-support').install({ errorFormatterForce: true })
const VersionRegistryContract = artifacts.require('VersionRegistry')
Expand Down Expand Up @@ -135,12 +138,14 @@ context('#ServerConfigParams', () => {

it('should fail on invalid relayhub address', async () => {
const config = { relayHubAddress: '123' }
await expectRevert(resolveServerConfig(config, provider), 'Provided address "123" is invalid, the capitalization checksum test failed, or its an indrect IBAN address which can\'t be converted')
await expectRevert(resolveServerConfig(config, provider),
'Provided address "123" is invalid, the capitalization checksum test failed, or its an indrect IBAN address which can\'t be converted')
})

it('should fail on no-contract relayhub address', async () => {
const config = { relayHubAddress: addr(1) }
await expectRevert(resolveServerConfig(config, provider), 'RelayHub: no contract at address 0x1111111111111111111111111111111111111111')
await expectRevert(resolveServerConfig(config, provider),
'RelayHub: no contract at address 0x1111111111111111111111111111111111111111')
})

it('should fail on missing hubid for VersionRegistry', async () => {
Expand All @@ -150,7 +155,8 @@ context('#ServerConfigParams', () => {

it('should fail on no-contract VersionRegistry address', async () => {
const config = { versionRegistryAddress: addr(1), relayHubId: 'hubid' }
await expectRevert(resolveServerConfig(config, provider), 'Invalid param versionRegistryAddress: no contract at address 0x1111111111111111111111111111111111111111')
await expectRevert(resolveServerConfig(config, provider),
'Invalid param versionRegistryAddress: no contract at address 0x1111111111111111111111111111111111111111')
})

contract('with VersionRegistry', () => {
Expand All @@ -165,12 +171,56 @@ context('#ServerConfigParams', () => {

it('should fail on invalid hub address in oracle', async () => {
const config = { versionRegistryAddress: oracle.address, relayHubId: 'hub-invalidaddr' }
await expectRevert(resolveServerConfig(config, provider), 'Invalid param relayHubId hub-invalidaddr @ 1.0: not an address: garbagevalue')
await expectRevert(resolveServerConfig(config, provider),
'Invalid param relayHubId hub-invalidaddr @ 1.0: not an address: garbagevalue')
})

it('should fail on no contract at hub address in oracle', async () => {
const config = { versionRegistryAddress: oracle.address, relayHubId: 'hub-nocontract' }
await expectRevert(resolveServerConfig(config, provider), 'RelayHub: no contract at address 0x2222222222222222222222222222222222222222')
await expectRevert(resolveServerConfig(config, provider),
'RelayHub: no contract at address 0x2222222222222222222222222222222222222222')
})
})

contract('Mandatory parameters', () => {
let hub: RelayHubInstance
before(async () => {
hub = await deployHub(constants.ZERO_ADDRESS, constants.ZERO_ADDRESS)
})

it('should fail on missing url', async () => {
const config = { relayHubAddress: hub.address }
await expectRevert(resolveServerConfig(config, provider), 'missing param: url')
})

it('should fail on missing workdir', async () => {
const config = { relayHubAddress: hub.address, url: 'fake.url.com' }
await expectRevert(resolveServerConfig(config, provider), 'missing param: workdir')
})

it('should fail on missing owner address', async () => {
const config = { relayHubAddress: hub.address, url: 'fake.url.com', workdir: '/path/to/somewhere/' }
await expectRevert(resolveServerConfig(config, provider), 'missing param: ownerAddress')
})

it('should fail on zero owner address', async () => {
const config = {
relayHubAddress: hub.address,
url: 'fake.url.com',
workdir: '/path/to/somewhere/',
ownerAddress: constants.ZERO_ADDRESS
}
await expectRevert(resolveServerConfig(config, provider), 'missing param: ownerAddress')
})

it('should succeed on valid config', async () => {
const config = {
relayHubAddress: hub.address,
url: 'fake.url.com',
workdir: '/path/to/somewhere/',
ownerAddress: '0x1111111111111111111111111111111111111111'
}
await resolveServerConfig(config, provider)
})
})
})
Expand Down
1 change: 0 additions & 1 deletion packages/dev/test/ServerTestEnvironment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,6 @@ export class ServerTestEnvironment {
// initialize server - gas price, stake, owner, etc, whatever
let latestBlock = await this.web3.eth.getBlock('latest')

// This run should call 'setOwner'
await this.relayServer._worker(latestBlock.number)
latestBlock = await this.web3.eth.getBlock('latest')
await this.stakeAndAuthorizeHub(ether('1'), unstakeDelay)
Expand Down
3 changes: 2 additions & 1 deletion packages/dev/test/penalizer/PenalizerService.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,8 @@ contract('PenalizerService', function (accounts) {
url: '',
workdir: '',
etherscanApiUrl: 'etherscanApiUrl',
relayHubAddress: env.relayHub.address
relayHubAddress: env.relayHub.address,
Copy link
Member

Choose a reason for hiding this comment

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

yes, its required... how did the test pass without it ?

Copy link
Member Author

Choose a reason for hiding this comment

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

It was given default owner (zero address), which is now no longer allowed. This is from previous PR #665

ownerAddress: env.relayServer.config.ownerAddress
}, web3.currentProvider) as ServerConfigParams
penalizerService = new PenalizerService(penalizerParams, logger, serverConfigParams)
await penalizerService.init(false)
Expand Down
2 changes: 1 addition & 1 deletion packages/paymasters/test/HashcashPaymaster.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ contract('HashcashPaymaster', ([from]) => {
})

it('should succeed with proper difficulty', async function () {
this.timeout(35000)
this.timeout(60000)

const input: GSNUnresolvedConstructorInput = {
provider: web3.currentProvider as HttpProvider,
Expand Down
19 changes: 19 additions & 0 deletions packages/relay/src/HttpServer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,10 @@ export class HttpServer {
// used to work before workspaces, needs research
// eslint-disable-next-line @typescript-eslint/no-misused-promises
this.app.get('/getaddr', this.pingHandler.bind(this))
// eslint-disable-next-line @typescript-eslint/no-misused-promises
this.app.post('/stats', this.statsHandler.bind(this))
// eslint-disable-next-line @typescript-eslint/no-misused-promises
this.app.get('/stats', this.statsHandler.bind(this))
// used to work before workspaces, needs research
// eslint-disable-next-line @typescript-eslint/no-misused-promises
this.app.post('/relay', this.relayHandler.bind(this))
Expand Down Expand Up @@ -85,6 +89,21 @@ export class HttpServer {
}
}

statsHandler (req: Request, res: Response): void {
if (this.relayService == null) {
throw new Error('RelayServer not initialized')
}
try {
const statsResponse = this.relayService.statsHandler()
res.send(statsResponse)
console.log('stats sent.')
Copy link
Member

Choose a reason for hiding this comment

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

logger.debug

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

} catch (e) {
const message: string = e.message
res.send({ message })
this.logger.error(`stats handler rejected: ${message}`)
}
}

async relayHandler (req: Request, res: Response): Promise<void> {
if (this.relayService == null) {
throw new Error('RelayServer not initialized')
Expand Down
26 changes: 26 additions & 0 deletions packages/relay/src/RelayServer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import { ServerAction } from './StoredTransaction'
import { TxStoreManager } from './TxStoreManager'
import { configureServer, ServerConfigParams, ServerDependencies } from './ServerConfigParams'
import { toBuffer } from 'ethereumjs-util'
import { ReadinessInfo, StatsResponse } from '@opengsn/common/dist/StatsResponse'
import Timeout = NodeJS.Timeout

/**
Expand Down Expand Up @@ -71,6 +72,7 @@ export class RelayServer extends EventEmitter {
config: ServerConfigParams
transactionManager: TransactionManager
txStoreManager: TxStoreManager
readinessInfo: ReadinessInfo

lastMinedActiveTransaction?: EventData

Expand Down Expand Up @@ -103,6 +105,14 @@ export class RelayServer extends EventEmitter {
}
this.reputationManager = dependencies.reputationManager
}
const now = Date.now()
this.readinessInfo = {
runningSince: now,
currentStateTimestamp: now,
totalReadyTime: 0,
totalNotReadyTime: 0,
totalReadinessChanges: 0
}
this.printServerAddresses()
this.logger.warn(`RelayServer version', ${gsnRuntimeVersion}`)
this.logger.info(`Using server configuration:\n ${JSON.stringify(this.config)}`)
Expand Down Expand Up @@ -138,6 +148,17 @@ export class RelayServer extends EventEmitter {
}
}

statsHandler (): StatsResponse {
const now = Date.now()
const statsResponse: StatsResponse = { ...this.readinessInfo, totalUptime: now - this.readinessInfo.runningSince }
if (this.isReady()) {
Copy link
Member

Choose a reason for hiding this comment

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

took me a while to understand: you update here the partial state, since last state change... maybe we can put a comment.)

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

statsResponse.totalReadyTime = this.readinessInfo.totalReadyTime + now - this.readinessInfo.currentStateTimestamp
} else {
statsResponse.totalNotReadyTime = this.readinessInfo.totalNotReadyTime + now - this.readinessInfo.currentStateTimestamp
}
return statsResponse
}

validateInput (req: RelayTransactionRequest, currentBlockNumber: number): void {
// Check that the relayHub is the correct one
if (req.metadata.relayHubAddress !== this.relayHubContract.address) {
Expand Down Expand Up @@ -791,16 +812,21 @@ latestBlock timestamp | ${latestBlock.timestamp}

setReadyState (isReady: boolean): void {
if (this.isReady() !== isReady) {
const now = Date.now()
if (isReady) {
if (this.lastSuccessfulRounds < this.config.successfulRoundsForReady) {
const roundsUntilReady = this.config.successfulRoundsForReady - this.lastSuccessfulRounds
this.logger.warn(chalk.yellow(`Relayer state: almost READY (in ${roundsUntilReady} rounds)`))
} else {
this.logger.warn(chalk.greenBright('Relayer state: READY'))
}
this.readinessInfo.totalNotReadyTime += now - this.readinessInfo.currentStateTimestamp
} else {
this.readinessInfo.totalReadyTime += now - this.readinessInfo.currentStateTimestamp
this.logger.warn(chalk.redBright('Relayer state: NOT-READY'))
}
this.readinessInfo.currentStateTimestamp = now
this.readinessInfo.totalReadinessChanges++
}
this.ready = isReady
}
Expand Down
1 change: 1 addition & 0 deletions packages/relay/src/ServerConfigParams.ts
Original file line number Diff line number Diff line change
Expand Up @@ -329,6 +329,7 @@ export async function resolveServerConfig (config: Partial<ServerConfigParams>,
}
if (config.url == null) error('missing param: url')
if (config.workdir == null) error('missing param: workdir')
if (config.ownerAddress == null || config.ownerAddress === constants.ZERO_ADDRESS) error('missing param: ownerAddress')
return { ...serverDefaultConfiguration, ...config }
}

Expand Down