-
Notifications
You must be signed in to change notification settings - Fork 22
improve error handling, buffer operations, and TypeScript configurat… #103
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -20,43 +20,52 @@ type PointG1 = typeof bls.G1.ProjectivePoint.ZERO | |
type PointG2 = typeof bls.G2.ProjectivePoint.ZERO | ||
|
||
async function verifyBeacon(chainInfo: ChainInfo, beacon: RandomnessBeacon, expectedRound: number): Promise<boolean> { | ||
const publicKey = chainInfo.public_key | ||
|
||
if (beacon.round !== expectedRound) { | ||
console.error('round was not the expected round') | ||
return false | ||
} | ||
|
||
if (!await randomnessIsValid(beacon)) { | ||
console.error('randomness did not match the signature') | ||
return false | ||
} | ||
|
||
if (isChainedBeacon(beacon, chainInfo)) { | ||
return bls.verify(beacon.signature, await chainedBeaconMessage(beacon), publicKey) | ||
} | ||
|
||
if (isUnchainedBeacon(beacon, chainInfo)) { | ||
return bls.verify(beacon.signature, await unchainedBeaconMessage(beacon), publicKey) | ||
} | ||
|
||
if (isG1G2SwappedBeacon(beacon, chainInfo)) { | ||
return verifySigOnG1(beacon.signature, await unchainedBeaconMessage(beacon), publicKey) | ||
try { | ||
if (!chainInfo || !chainInfo.public_key) { | ||
throw new Error('Invalid chain info: Missing public key'); | ||
} | ||
|
||
const publicKey = chainInfo.public_key; | ||
|
||
if (!beacon || typeof beacon.round !== 'number') { | ||
throw new Error('Invalid beacon: Missing or invalid round number'); | ||
} | ||
|
||
if (beacon.round !== expectedRound) { | ||
throw new Error(`Round mismatch: Expected ${expectedRound}, got ${beacon.round}`); | ||
} | ||
|
||
if (!await randomnessIsValid(beacon)) { | ||
throw new Error('Invalid beacon: Randomness does not match signature'); | ||
} | ||
|
||
if (isChainedBeacon(beacon, chainInfo)) { | ||
return bls.verify(beacon.signature, await chainedBeaconMessage(beacon), publicKey); | ||
} | ||
|
||
if (isUnchainedBeacon(beacon, chainInfo)) { | ||
return bls.verify(beacon.signature, await unchainedBeaconMessage(beacon), publicKey); | ||
} | ||
|
||
if (isG1G2SwappedBeacon(beacon, chainInfo)) { | ||
return verifySigOnG1(beacon.signature, await unchainedBeaconMessage(beacon), publicKey); | ||
} | ||
|
||
if (isG1Rfc9380(beacon, chainInfo)) { | ||
return verifySigOnG1(beacon.signature, await unchainedBeaconMessage(beacon), publicKey, 'BLS_SIG_BLS12381G1_XMD:SHA-256_SSWU_RO_NUL_'); | ||
} | ||
|
||
if (isBn254OnG1(beacon, chainInfo)) { | ||
return bn254.verifyShortSignature(beacon.signature, await unchainedBeaconMessage(beacon, keccak_256), publicKey, { | ||
DST: 'BLS_SIG_BN254G1_XMD:KECCAK-256_SVDW_RO_NUL_' | ||
}); | ||
} | ||
|
||
throw new Error(`Unsupported beacon type: ${chainInfo.schemeID}`); | ||
} catch (error) { | ||
console.error('Beacon verification failed:', error instanceof Error ? error.message : 'Unknown error'); | ||
return false; | ||
} | ||
|
||
if (isG1Rfc9380(beacon, chainInfo)) { | ||
return verifySigOnG1(beacon.signature, await unchainedBeaconMessage(beacon), publicKey, 'BLS_SIG_BLS12381G1_XMD:SHA-256_SSWU_RO_NUL_') | ||
} | ||
|
||
if (isBn254OnG1(beacon, chainInfo)) { | ||
return bn254.verifyShortSignature(beacon.signature, await unchainedBeaconMessage(beacon, keccak_256), publicKey, { | ||
DST: 'BLS_SIG_BN254G1_XMD:KECCAK-256_SVDW_RO_NUL_' | ||
}) | ||
} | ||
|
||
console.error(`Beacon type ${chainInfo.schemeID} was not supported or the beacon was not of the purported type`) | ||
return false | ||
|
||
} | ||
|
||
// @noble/curves/bls12-381 has not yet implemented public keys on G2, so we've implemented a manual verification for beacons on G1 | ||
|
@@ -110,9 +119,16 @@ function signatureBuffer(sig: string) { | |
} | ||
|
||
function roundBuffer(round: number) { | ||
const buffer = Buffer.alloc(8) | ||
buffer.writeBigUInt64BE(BigInt(round)) | ||
return buffer | ||
if (!Number.isInteger(round) || round < 0) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the check here makes sense, though I dunno the rationale behind replacing |
||
throw new Error('Round number must be a non-negative integer'); | ||
} | ||
const buffer = Buffer.alloc(8); | ||
let value = BigInt(round); | ||
for (let i = 7; i >= 0; i--) { | ||
buffer[i] = Number(value & BigInt(0xFF)); | ||
value = value >> BigInt(8); | ||
} | ||
return buffer; | ||
} | ||
|
||
async function randomnessIsValid(beacon: RandomnessBeacon): Promise<boolean> { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,41 +1,61 @@ | ||
import {Chain, ChainInfo, ChainOptions, ChainVerificationParams, defaultChainOptions} from './index' | ||
import {HttpOptions, jsonOrError} from './util' | ||
import {Chain, ChainClient, ChainOptions, defaultChainOptions, RandomnessBeacon} from './index' | ||
import {defaultHttpOptions, HttpOptions, jsonOrError} from './util' | ||
|
||
class HttpChainClient implements ChainClient { | ||
private readonly baseUrl: string; | ||
|
||
class HttpChain implements Chain { | ||
constructor( | ||
public baseUrl: string, | ||
private options: ChainOptions = defaultChainOptions, | ||
private httpOptions: HttpOptions = {}) { | ||
private someChain: Chain, | ||
public options: ChainOptions = defaultChainOptions, | ||
public httpOptions: HttpOptions = { | ||
...defaultHttpOptions, | ||
timeout: 10000, // 10 second timeout | ||
} | ||
) { | ||
if (!someChain || !someChain.baseUrl) { | ||
throw new Error('Invalid chain: Missing base URL'); | ||
} | ||
this.baseUrl = someChain.baseUrl.replace(/\/+$/, ''); // Remove trailing slashes | ||
} | ||
|
||
async info(): Promise<ChainInfo> { | ||
const chainInfo = await jsonOrError(`${this.baseUrl}/info`, this.httpOptions) | ||
if (!!this.options.chainVerificationParams && !isValidInfo(chainInfo, this.options.chainVerificationParams)) { | ||
throw Error(`The chain info retrieved from ${this.baseUrl} did not match the verification params!`) | ||
async get(roundNumber: number): Promise<RandomnessBeacon> { | ||
if (!Number.isInteger(roundNumber) || roundNumber < 1) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we actually accept |
||
throw new Error('Invalid round number: Must be a positive integer'); | ||
} | ||
return chainInfo | ||
} | ||
} | ||
|
||
function isValidInfo(chainInfo: ChainInfo, validParams: ChainVerificationParams): boolean { | ||
return chainInfo.hash === validParams.chainHash && chainInfo.public_key === validParams.publicKey | ||
} | ||
try { | ||
const url = withCachingParams(`${this.baseUrl}/public/${roundNumber}`, this.options); | ||
return await jsonOrError(url, this.httpOptions); | ||
} catch (error) { | ||
throw new Error(`Failed to fetch round ${roundNumber}: ${error instanceof Error ? error.message : 'Unknown error'}`); | ||
} | ||
} | ||
|
||
class HttpCachingChain implements Chain { | ||
private chain: Chain | ||
private cachedInfo?: ChainInfo | ||
async latest(): Promise<RandomnessBeacon> { | ||
try { | ||
const url = withCachingParams(`${this.baseUrl}/public/latest`, this.options); | ||
return await jsonOrError(url, this.httpOptions); | ||
} catch (error) { | ||
throw new Error(`Failed to fetch latest beacon: ${error instanceof Error ? error.message : 'Unknown error'}`); | ||
} | ||
} | ||
|
||
constructor(public baseUrl: string, private options: ChainOptions = defaultChainOptions) { | ||
this.chain = new HttpChain(baseUrl, options) | ||
chain(): Chain { | ||
return this.someChain; | ||
} | ||
} | ||
|
||
async info(): Promise<ChainInfo> { | ||
if (!this.cachedInfo) { | ||
this.cachedInfo = await this.chain.info() | ||
} | ||
return this.cachedInfo | ||
function withCachingParams(url: string, config: ChainOptions): string { | ||
if (!url) { | ||
throw new Error('Invalid URL: URL cannot be empty'); | ||
} | ||
|
||
if (config.noCache) { | ||
const timestamp = Date.now(); | ||
const separator = url.includes('?') ? '&' : '?'; | ||
return `${url}${separator}nocache=${timestamp}`; | ||
} | ||
return url; | ||
} | ||
|
||
export {HttpChain} | ||
export default HttpCachingChain | ||
export default HttpChainClient |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -68,16 +68,25 @@ export interface ChainClient { | |
|
||
// fetch a beacon for a given `roundNumber` or get the latest beacon by omitting the `roundNumber` | ||
export async function fetchBeacon(client: ChainClient, roundNumber?: number): Promise<RandomnessBeacon> { | ||
if (!roundNumber) { | ||
roundNumber = roundAt(Date.now(), await client.chain().info()) | ||
try { | ||
if (!roundNumber) { | ||
const info = await client.chain().info(); | ||
roundNumber = roundAt(Date.now(), info); | ||
} | ||
|
||
if (roundNumber < 1) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 0 accepted too There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. hmm although the old code didn't accept it 🤔 |
||
throw new Error('Invalid round number: Cannot request lower than round number 1'); | ||
} | ||
|
||
const beacon = await client.get(roundNumber); | ||
if (!beacon) { | ||
throw new Error(`Failed to fetch beacon for round ${roundNumber}`); | ||
} | ||
|
||
return validatedBeacon(client, beacon, roundNumber); | ||
} catch (error) { | ||
throw new Error(`Failed to fetch beacon: ${error instanceof Error ? error.message : 'Unknown error'}`); | ||
} | ||
|
||
if (roundNumber < 1) { | ||
throw Error('Cannot request lower than round number 1') | ||
} | ||
const beacon = await client.get(roundNumber) | ||
|
||
return validatedBeacon(client, beacon, roundNumber) | ||
} | ||
|
||
// fetch the most recent beacon to have been emitted at a given `time` in epoch ms | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,64 +6,107 @@ import {LIB_VERSION} from './version' | |
|
||
export function sleep(timeMs: number): Promise<void> { | ||
return new Promise(resolve => { | ||
if (!Number.isFinite(timeMs)) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. bit of a random change - this is only used a few places in the lib itself, none of which take |
||
throw new Error('Invalid sleep time: Must be a finite number'); | ||
} | ||
if (timeMs <= 0) { | ||
resolve() | ||
resolve(); | ||
return; | ||
} | ||
setTimeout(resolve, timeMs) | ||
}) | ||
setTimeout(resolve, timeMs); | ||
}); | ||
} | ||
|
||
export function roundAt(time: number, chain: ChainInfo) { | ||
export function roundAt(time: number, chain: ChainInfo): number { | ||
if (!Number.isFinite(time)) { | ||
throw new Error('Cannot use Infinity or NaN as a beacon time') | ||
throw new Error('Invalid time: Must be a finite number'); | ||
} | ||
if (!chain || !chain.genesis_time || !chain.period) { | ||
throw new Error('Invalid chain info: Missing required fields'); | ||
} | ||
if (time < chain.genesis_time * 1000) { | ||
throw Error('Cannot request a round before the genesis time') | ||
|
||
const genesisTimeMs = chain.genesis_time * 1000; | ||
if (time < genesisTimeMs) { | ||
throw new Error(`Invalid time: Cannot request a round before the genesis time (${new Date(genesisTimeMs).toISOString()})`); | ||
} | ||
return Math.floor((time - (chain.genesis_time * 1000)) / (chain.period * 1000)) + 1 | ||
|
||
return Math.floor((time - genesisTimeMs) / (chain.period * 1000)) + 1; | ||
} | ||
|
||
export function roundTime(chain: ChainInfo, round: number) { | ||
export function roundTime(chain: ChainInfo, round: number): number { | ||
if (!Number.isFinite(round)) { | ||
throw new Error('Cannot use Infinity or NaN as a round number') | ||
throw new Error('Invalid round: Must be a finite number'); | ||
} | ||
round = round < 0 ? 0 : round | ||
return (chain.genesis_time + (round - 1) * chain.period) * 1000 | ||
if (!chain || !chain.genesis_time || !chain.period) { | ||
throw new Error('Invalid chain info: Missing required fields'); | ||
} | ||
|
||
round = Math.max(0, round); | ||
return (chain.genesis_time + (round - 1) * chain.period) * 1000; | ||
} | ||
|
||
export type HttpOptions = { | ||
userAgent?: string | ||
headers?: Record<string, string> | ||
} | ||
userAgent?: string; | ||
headers?: Record<string, string>; | ||
timeout?: number; | ||
}; | ||
|
||
// taking a separate `userAgent` param for backwards compatibility | ||
export const defaultHttpOptions: HttpOptions = { | ||
userAgent: `drand-client-${LIB_VERSION}`, | ||
} | ||
timeout: 30000, // 30 second default timeout | ||
}; | ||
|
||
// eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
export async function jsonOrError(url: string, options: HttpOptions = defaultHttpOptions): Promise<any> { | ||
const headers = {...options.headers} | ||
export async function jsonOrError<T>(url: string, options: HttpOptions = defaultHttpOptions): Promise<T> { | ||
if (!url || typeof url !== 'string') { | ||
throw new Error('Invalid URL: Must be a non-empty string'); | ||
} | ||
|
||
const headers: Record<string, string> = {...(options.headers || {})}; | ||
if (options.userAgent) { | ||
headers['User-Agent'] = options.userAgent | ||
headers['User-Agent'] = options.userAgent; | ||
} | ||
|
||
const response = await fetch(url, {headers}) | ||
if (!response.ok) { | ||
throw Error(`Error response fetching ${url} - got ${response.status}`) | ||
} | ||
try { | ||
const controller = new AbortController(); | ||
const timeoutId = setTimeout(() => controller.abort(), options.timeout || defaultHttpOptions.timeout); | ||
|
||
return await response.json() | ||
const response = await fetch(url, { | ||
headers, | ||
signal: controller.signal | ||
}); | ||
|
||
clearTimeout(timeoutId); | ||
|
||
if (!response.ok) { | ||
throw new Error(`HTTP ${response.status}: ${response.statusText}`); | ||
} | ||
|
||
return await response.json(); | ||
} catch (error) { | ||
if (error instanceof Error) { | ||
if (error.name === 'AbortError') { | ||
throw new Error(`Request timeout after ${options.timeout || defaultHttpOptions.timeout}ms`); | ||
} | ||
throw new Error(`Failed to fetch ${url}: ${error.message}`); | ||
} | ||
throw new Error(`Failed to fetch ${url}: Unknown error`); | ||
} | ||
} | ||
|
||
export async function retryOnError<T>(fn: () => Promise<T>, times: number): Promise<T> { | ||
export async function retryOnError<T>(fn: () => Promise<T>, times: number, delayMs: number = 1000): Promise<T> { | ||
if (!Number.isInteger(times) || times < 0) { | ||
throw new Error('Invalid retry count: Must be a non-negative integer'); | ||
} | ||
|
||
try { | ||
return await fn() | ||
} catch (err) { | ||
return await fn(); | ||
} catch (error) { | ||
if (times === 0) { | ||
throw err | ||
throw error; | ||
} | ||
return retryOnError(fn, times - 1) | ||
|
||
await sleep(delayMs); | ||
return retryOnError(fn, times - 1, delayMs); | ||
} | ||
} |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
catching locally thrown errors feels like a bit of a code smell. Also this could be a breaking change for users expecting certain cryptographic errors to throw exceptions