Ask about publication gaps in the rate provider review

.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?

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? \<YES: minimum duration/NO\>
 
 ### 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). \<Delete this hint: If unchecked, delete all of the bullets below.\>
     - source: \<Delete this hint: If Chainlink, just write "Chainlink". Otherwise, elaborate.\>
     - source address: [\<network:address\>](\<link to contract block explorer\>)
     - any protections? \<YES: elaborate/NO\> \<Delete this hint: e.g., maximum rate delta\>
 
+- [ ] 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). \<Delete this hint: If unchecked, record below where freshness is enforced, then delete the remaining bullets.\>
+    - where freshness is enforced: \<Rate Provider/source contract/nowhere\>
+    - maximum age, if any: \<duration/none\>
+    - expected update cadence: \<duration\> \<Delete this hint: e.g., a published Chainlink heartbeat, or an interval measured on chain\>
+
+- [ ] The source stops publishing on a schedule rather than updating continuously (e.g., it follows market hours, or it publishes a periodic valuation). \<Delete this hint: If unchecked, delete all of the bullets below.\>
+    - publication pattern: \<Delete this hint: e.g., US equity hours on weekdays; or a net asset value marked three times a week\>
+    - longest routine gap: \<duration\> \<Delete this hint: measure it on chain rather than assuming; e.g., about 65 hours from a Friday close to a Monday open, or about 98 hours between periodic marks\>
+    - is the published value a live market quote, or a periodic valuation? \<quote/valuation\>
+    - while the source is silent, can the value move discontinuously? \<YES: elaborate/NO\> \<Delete this hint: this is the bullet that decides whether the gap is a risk or just how the source works. A quote can gap when the venue reopens. A smooth accrual, such as a net asset value that has only ever risen, cannot: the stale value is understated by a knowable and small amount.\>
+    - does the source publish a market state or session field? \<YES: name the field and its values/NO\>
+    - what does `getRate` return during a gap? \<the last published value, served as though current/reverts\>
+
 - [ ] Price data is expected to be volatile (e.g., because it represents an open market price instead of a (mostly) monotonically increasing price). \<Delete this hint: If unchecked, delete all of the bullets below.\>
     - description: \<`X` in terms of `Y`\> \<Delete this hint: e.g., `wstETH` in terms of `ETH`\>
     - should be: \<`A` in terms of `B`\> \<Delete this hint: e.g., `wstETH` in terms of `stETH`\>

scripts/process-issue.ts

@@ -97,16 +97,24 @@ 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,
         issueData.asset_contract_address,
         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

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 <address> --network <network> --rateProviderAsset <asset> --rpcUrl <rpcUrl>
+// npm run write-review -- --rateProviderAddress <address> --network <network> --rateProviderAsset <asset> --rpcUrl <rpcUrl> --isStablecoin <true|false> --publicationGaps <true|false>
 
 // 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

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}}**
 

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 =
+    '\\<to be completed: where freshness is enforced (Rate Provider/source contract/nowhere), the maximum age if any, and the expected update cadence\\>'
+
+// 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: \\<to be completed\\>
+    - longest routine gap: \\<to be completed: measure it on chain rather than assuming\\>
+    - is the published value a live market quote, or a periodic valuation? \\<to be completed\\>
+    - while the source is silent, can the value move discontinuously? \\<to be completed: this decides whether the gap is a risk or just how the source works\\>
+    - how far did the value actually move across the longest gap? \\<to be completed: record the movement next to the gap; a long gap on a value that barely moves is not the same risk as a short gap on one that does\\>
+    - does the source publish a market state or session field? \\<to be completed: name the field and its values, or NO\\>
+    - what does \\\`getRate\\\` return during a gap? \\<to be completed: the last published value served as though current, or reverts\\>
+`
+
 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,