diff --git a/.github/ISSUE_TEMPLATE/automated-review-request.yml b/.github/ISSUE_TEMPLATE/automated-review-request.yml index c98327e..c2449d8 100644 --- a/.github/ISSUE_TEMPLATE/automated-review-request.yml +++ b/.github/ISSUE_TEMPLATE/automated-review-request.yml @@ -12,6 +12,7 @@ body: options: - label: Is the rate provider reporting for a stable coin/FX/Yield tracking? - label: Is the rate provider reporting a market rate? + - label: Does the price source stop publishing on a schedule (market hours, or a periodic valuation) rather than updating continuously? - label: Is a combined rate provider needed (i.e. wstETH in aave)? - label: Does this rate provider pertain to an ERC4626 contract? - label: If so, is the intention for this ERC4626 asset to be boosted? diff --git a/rate-providers/template.md b/rate-providers/template.md index 8754f18..27d0686 100644 --- a/rate-providers/template.md +++ b/rate-providers/template.md @@ -40,11 +40,47 @@ If none of these is checked, then this might be a pretty great Rate Provider! If - multisig timelock? \ ### Oracles + +A note on freshness, because the two questions below look like one, but are actually separate. An elapsed-time bound alone cannot be set for a source that stops publishing on a schedule: any maximum age short enough to catch a source that failed between publications also rejects every routine gap, and any bound loose enough to survive a routine gap is far too loose to catch a failure. + +The asset class by itself does not decide the answer, since they may behave very differently. A tokenized real-world asset can belong to any of the three categories below. Determine which one by looking at what the source actually published over the last week, not just by the asset type. + +| Category | When it publishes | What the value does while the source is silent | Is the gap a risk? | +|---|---|---|---| +| Live market quote on a venue calendar | during venue hours only, silent overnight and at weekends | can gap when the venue reopens, because the market re-prices without the source | Yes. This is the case the questions below exist for. | +| Periodic valuation, such as a net asset value mark | on its own schedule, which can mean gaps of several days | accrues smoothly, so the stale value is understated by a small and knowable amount | No. The gap is how the source works, not a defect. A maximum-age bound would reject the price during normal operation. | +| Real-world asset priced continuously | without interruption, including weekends | keeps being re-marked, so there is no scheduled gap at all | Not applicable. Leave the item below unchecked, and record the ordinary freshness answer above instead. | + +The third category is the one most often misread: an asset can be a tokenized bond or equity, yet still have a source that never stops publishing. Check the publication record before assuming a calendar applies. + +Sources from each category were measured on chain for calibration. + +A live quote for a short-duration bond fund on a US equity venue published a session field that cycled through overnight, pre-market, regular, and post-market, updating about every 600 seconds in all four, then held a single closing mark from Saturday 00:00 UTC onward. By the following Monday's open that mark was about 61 hours old. Because all four trading sessions are used, a provider that accepts them all stays live around the clock on weekdays and only stops at weekends. + +A fund net asset value mark published 23 times over 66 days, with routine gaps of 24, 46 to 49, and 91 to 98 hours. This tracks a slowly increasing NAV. Every one of its 23 moves was positive, between 0.013 and 0.22 percent, averaging about 0.03 percent a day. A maximum-age bound under about 120 hours would have rejected this price during normal operation, which is the clearest illustration of why this item asks about publication behavior instead of demanding a number. + +A tokenized treasury bill kept publishing straight through a weekend, moving by a single quantization step of the feed. Its asset class suggested a market calendar: but its publication record showed none. + +So, the exposure from a stale price is bounded by how far the value moves while the source is silent. The bond fund quote above moved about 0.005 percent across a full trading day. A gap is only worth worrying about when the value behind it can move meaningfully, or discontinuously, while nobody is publishing. Record the movement alongside the gap, because the gap on its own does not tell you whether there is a problem. + - [ ] Price data is provided by an off-chain source (e.g., a Chainlink oracle, a multisig, or a network of nodes). \ - source: \ - source address: [\](\) - any protections? \ \ +- [ ] The rate can go stale without anything rejecting it (i.e., neither the Rate Provider nor its source refuses a price that has stopped updating). \ + - where freshness is enforced: \ + - maximum age, if any: \ + - expected update cadence: \ \ + +- [ ] The source stops publishing on a schedule rather than updating continuously (e.g., it follows market hours, or it publishes a periodic valuation). \ + - publication pattern: \ + - longest routine gap: \ \ + - is the published value a live market quote, or a periodic valuation? \ + - while the source is silent, can the value move discontinuously? \ \ + - does the source publish a market state or session field? \ + - what does `getRate` return during a gap? \ + - [ ] Price data is expected to be volatile (e.g., because it represents an open market price instead of a (mostly) monotonically increasing price). \ - description: \<`X` in terms of `Y`\> \ - should be: \<`A` in terms of `B`\> \ diff --git a/scripts/process-issue.ts b/scripts/process-issue.ts index b0b0833..d118274 100644 --- a/scripts/process-issue.ts +++ b/scripts/process-issue.ts @@ -97,6 +97,16 @@ async function processIssue(issueJson: string) { }, } + // These strings must match the checkbox labels in .github/ISSUE_TEMPLATE/automated-review-request.yml + // exactly. A label that no longer matches silently reads as unselected rather than failing. + const selected = issueData.additional_contract_information.selected + const warnings = { + isMarketRate: selected.includes('Is the rate provider reporting a market rate?'), + hasPublicationGaps: selected.includes( + 'Does the price source stop publishing on a schedule (market hours, or a periodic valuation) rather than updating continuously?', + ), + } + await writeReviewAndUpdateRegistry( issueData.rate_provider_contract_address, network, @@ -104,9 +114,7 @@ async function processIssue(issueJson: string) { rpcUrl as string, issueData.protocol_documentation, issueData.audits, - issueData.additional_contract_information.selected.includes('Is the rate provider reporting a market rate?') - ? { isMarketRate: true } - : undefined, + warnings, ) // this step requires the registry to be read thus having the registry updated already diff --git a/scripts/write-review.ts b/scripts/write-review.ts index 486853b..cdfe4a8 100644 --- a/scripts/write-review.ts +++ b/scripts/write-review.ts @@ -30,7 +30,7 @@ const fs = require('fs') // to use this script use the command below // for network see the viem chains // Important: The custom RPC URL in the .env must support createAccessList (or the viem default rpc url) -// npm run write-review -- --rateProviderAddress
--network --rateProviderAsset --rpcUrl +// npm run write-review -- --rateProviderAddress
--network --rateProviderAsset --rpcUrl --isStablecoin --publicationGaps // Parse command-line arguments using yargs async function main() { @@ -73,6 +73,13 @@ async function main() { description: 'The asset the rate provider provides the rate for', demandOption: true, }) + .option('publicationGaps', { + alias: 'g', + type: 'string', + description: + "The price source stops publishing on a schedule rather than updating continuously, either following a venue calendar or publishing a periodic valuation. Read the source's publication record to answer this; the asset class does not decide it", + demandOption: true, + }) .option('rpcUrl', { alias: 'u', type: 'string', @@ -147,7 +154,11 @@ async function main() { throw new Error(`Invalid rateProviderAsset: ${argv.rateProviderAsset}. It must start with "0x".`) })() - await writeReviewAndUpdateRegistry(rateProviderAddress, network, rateProviderAsset, argv.rpcUrl) + const hasPublicationGaps = argv.publicationGaps === 'true' ? true : false + + await writeReviewAndUpdateRegistry(rateProviderAddress, network, rateProviderAsset, argv.rpcUrl, undefined, undefined, { + hasPublicationGaps, + }) // the registry file has been updated. All relevant information can be read from there and don't need to be passed as arguments const isStablecoin = argv.isStablecoin === 'true' ? true : false diff --git a/src/utils/template.ts b/src/utils/template.ts index f882717..2183da3 100644 --- a/src/utils/template.ts +++ b/src/utils/template.ts @@ -23,6 +23,13 @@ Each of the items below represents an absolute requirement for the Rate Provider - [{{hasUpgradeableElements}}] Other contracts which are part of the \`getRate\` callchain are upgradeable**. You can find more information about the involved contracts in this [tenderly simulation]({{tenderlySimUrl}}) +### Oracles +- [{{hasPublicationGaps}}] The source stops publishing on a schedule rather than updating continuously (e.g., it follows market hours, or it + publishes a periodic valuation). An elapsed-time staleness bound cannot be calibrated for such a source: any bound short enough to + catch a failure between publications also rejects every routine gap. Whether the gap is a risk depends on what the value does while + the source is silent. A live market quote can gap when the venue reopens; a smooth accrual cannot. +{{publicationGapsDetail}}- Freshness: {{freshnessNotes}} + ## Conclusion **Summary judgment: {{isUsable}}** diff --git a/src/utils/write-rp-review.ts b/src/utils/write-rp-review.ts index 14d31d7..f8fc695 100644 --- a/src/utils/write-rp-review.ts +++ b/src/utils/write-rp-review.ts @@ -38,9 +38,32 @@ const chainNameToRegistryKey: { [key: string]: string } = { } type RateProviderWarnings = { - isMarketRate: boolean + isMarketRate?: boolean + // The price source stops publishing on a schedule rather than updating continuously, either because it + // follows a venue calendar or because it publishes a periodic valuation. Determined from the source's + // publication record, not from the asset class: a tokenized real-world asset may publish continuously. + hasPublicationGaps?: boolean } +// Emitted in place of the freshness notes, which cannot be determined on chain. It is deliberately not a +// checkbox: an unchecked box in this checklist reads as "this red flag does not apply", which would assert +// that freshness was reviewed when nobody had looked at it. +const FRESHNESS_PLACEHOLDER = + '\\' + +// Follow-up questions emitted only when the source has scheduled publication gaps. A maximum age cannot +// answer these, because no single bound both survives a routine gap and catches a failure between +// publications. The decisive questions are what the value does while the source is silent, and how far it +// actually moved: a gap only matters when the value behind it can move meaningfully while nobody publishes. +const PUBLICATION_GAPS_DETAIL = ` - publication pattern: \\ + - longest routine gap: \\ + - is the published value a live market quote, or a periodic valuation? \\ + - while the source is silent, can the value move discontinuously? \\ + - how far did the value actually move across the longest gap? \\ + - does the source publish a market state or session field? \\ + - what does \\\`getRate\\\` return during a gap? \\ +` + export async function writeReviewAndUpdateRegistry( rateProviderAddress: Address, network: Chain, @@ -70,6 +93,7 @@ export async function writeReviewAndUpdateRegistry( isScale18: `${rateInfo.scale18 ? 'x' : ' '}`, isUpgradeable: `${upgradeData.map((c) => c.address).includes(rateProviderAddress) ? 'x' : ' '}`, hasUpgradeableElements: `${upgradeData.filter((contract) => contract.address !== rateProviderAddress).length > 0 ? 'x' : ' '}`, + hasPublicationGaps: `${warnings?.hasPublicationGaps ? 'x' : ' '}`, isUsable: `${hasInterfaceImplemented && rateInfo.scale18 ? 'USABLE' : 'UNUSABLE'}`, } @@ -90,6 +114,9 @@ export async function writeReviewAndUpdateRegistry( .replace('{{isScale18}}', templateData.isScale18) .replace('{{isUpgradeable}}', templateData.isUpgradeable) .replace('{{hasUpgradeableElements}}', templateData.hasUpgradeableElements) + .replace('{{hasPublicationGaps}}', templateData.hasPublicationGaps) + .replace('{{publicationGapsDetail}}', warnings?.hasPublicationGaps ? PUBLICATION_GAPS_DETAIL : '') + .replace('{{freshnessNotes}}', FRESHNESS_PLACEHOLDER) .replace('{{isUsable}}', templateData.isUsable) .replace('{{tenderlySimUrl}}', tenderlysimUrl) @@ -107,7 +134,10 @@ export async function writeReviewAndUpdateRegistry( name: `${(contractName.charAt(0).toUpperCase() + contractName.slice(1)).replace(' ', '')}RateProvider.md`, summary: templateData.isUsable === 'USABLE' ? 'safe' : 'unsafe', review: `./${(contractName.charAt(0).toUpperCase() + contractName.slice(1)).replace(' ', '')}RateProviderReview${shortUuid}.md`, - warnings: warnings?.isMarketRate ? ['market-rate'] : [], + warnings: [ + ...(warnings?.isMarketRate ? ['market-rate'] : []), + ...(warnings?.hasPublicationGaps ? ['publication-gaps'] : []), + ], factory: '', upgradeableComponents: upgradeData.map((contract) => ({ entrypoint: contract.address,