Skip to content

feat: consolidate per-protocol assertion examples under examples/#70

Open
makemake-kbo wants to merge 2 commits into
masterfrom
all-examples
Open

feat: consolidate per-protocol assertion examples under examples/#70
makemake-kbo wants to merge 2 commits into
masterfrom
all-examples

Conversation

@makemake-kbo
Copy link
Copy Markdown
Contributor

Brings assertion examples from the per-protocol branches (aave, aerodrome, cap, curve, denaria, euler, nado, royco, safe, spark, symbiotic, tydro, uniswap, veda, zeroex) into examples//src/ on a single branch.

Each protocol is compiled standalone via a new [profile.] in foundry.toml that shares the root src/ (credible-std) and lib/ via the credible-std/ remapping — no nested foundry projects, no duplicated submodules. Imports in extracted files are rewritten to the credible-std/ remapping form.

Root src/ is extended with the missing protection bases that examples depend on (access_control/*, perpetual/PerpetualBaseAssertion + IPerpetualProtectionSuite). AaveV3LikeOperationSafety is upgraded to the full version that exposes AaveV3LikeProtectionSuite. PhEvm gains the marketPrice precompile signature and IAaveV3LikeOracle gains BASE_CURRENCY / BASE_CURRENCY_UNIT.

All 20 foundry profiles build clean.

Brings assertion examples from the per-protocol branches (aave, aerodrome,
cap, curve, denaria, euler, nado, royco, safe, spark, symbiotic, tydro,
uniswap, veda, zeroex) into examples/<protocol>/src/ on a single branch.

Each protocol is compiled standalone via a new [profile.<protocol>] in
foundry.toml that shares the root src/ (credible-std) and lib/ via the
credible-std/ remapping — no nested foundry projects, no duplicated
submodules. Imports in extracted files are rewritten to the credible-std/
remapping form.

Root src/ is extended with the missing protection bases that examples
depend on (access_control/*, perpetual/PerpetualBaseAssertion +
IPerpetualProtectionSuite). AaveV3LikeOperationSafety is upgraded to the
full version that exposes AaveV3LikeProtectionSuite. PhEvm gains the
marketPrice precompile signature and IAaveV3LikeOracle gains
BASE_CURRENCY / BASE_CURRENCY_UNIT.

All 20 foundry profiles build clean.
@fredo
Copy link
Copy Markdown
Contributor

fredo commented May 26, 2026

Blocking concern: several new assertion examples appear to compile but never call registerAssertionSpec, so they may not be valid/deployable Credible assertions.

SpecRecorder.registerAssertionSpec documents that an assertion needs a defined spec to be valid (src/SpecRecorder.sol, around lines 23-26). Some examples follow that pattern, but I could not find a spec registration in several new concrete assertions or their shared bases, including:

  • examples/denaria/src/DenariaOperationSafety.sol / src/protection/perpetual/PerpetualBaseAssertion.sol
  • examples/spark/src/SparkVaultAssertion.sol / src/protection/vault/ERC4626BaseAssertion.sol
  • examples/spark/src/SparkLendOraclePriceGuardAssertion.sol
  • examples/spark/src/SparkSLLInflowStopgapAssertion.sol
  • examples/curve/src/LlamaLendVaultAssertion.sol
  • examples/euler/src/EulerEVaultAssertion.sol / examples/euler/src/EulerEVaultHelpers.sol
  • examples/royco/src/RoycoKernelAssertion.sol / examples/royco/src/RoycoHelpers.sol
  • examples/symbiotic/src/SymbioticVaultAssertion.sol / examples/symbiotic/src/SymbioticVaultBaseAssertion.sol
  • examples/veda/src/BoringVaultAssertion.sol / examples/veda/src/BoringVaultHelpers.sol

Can we either register the assertion spec in each concrete assertion constructor, or centralize it in the relevant base constructors where the trigger/target set is known? I would also add a small deployment/spec-recording test per affected example profile so this does not regress while the examples still compile.

Copy link
Copy Markdown
Contributor

@fredo fredo left a comment

Choose a reason for hiding this comment

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

Constructor-time protocol reads need to be removed before these examples are deployable as Credible assertions.

uint256 priceTolerance_
) {
amm = amm_;
borrowedToken = ICurveLlammaAMM(amm_).coins(0);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This constructor reads live AMM state (coins) and then token metadata (decimals) below. That has the same deployability problem documented in the ERC4626/Uniswap helpers: assertion construction runs in an isolated deploy context, so adopter/token reads can revert with EXTCODESIZE = 0. Please pass the borrowed token, collateral token, and precisions/decimals explicitly, or move reads to fork-time helpers using ph.staticcallAt.

Comment thread examples/curve/src/CurveUsdProtocol.sol Outdated

constructor(address controller_, uint256 maxLoansToScan_, uint256 debtTolerance_) {
controller = controller_;
amm = ICurveUsdController(controller_).amm();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Same constructor-read issue here: controller_.amm() reaches into the protocol while the assertion is being constructed. Please pass the AMM address explicitly, or derive it later via a fork-aware read.

address internal immutable controller;

constructor(address vault_) {
controller = ILlamaLendVault(vault_).controller();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Same issue: this constructor reads vault_.controller() from the adopter/protocol. Constructors should only consume deployment args for this kind of dependency; pass the controller explicitly or read it later through the fork-aware helpers.


constructor(address controller_, uint256 availableBalanceTolerance_) {
controller = controller_;
borrowedToken = ILlamaLendController(controller_).borrowed_token();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Same constructor-read issue: controller_.borrowed_token() depends on live protocol code during assertion deployment. Please pass the borrowed token explicitly or defer this through ph.staticcallAt at assertion execution time.

Comment thread examples/royco/src/RoycoHelpers.sol Outdated

constructor(address kernel_) {
kernel = kernel_;
accountant = IRoycoKernel(kernel_).ACCOUNTANT();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This constructor reads multiple values from the kernel (ACCOUNTANT, tranches, and assets). That can fail in the assertion-deploy runtime for the same reason as the other adopter reads. Please pass these dependencies explicitly or make them fork-time reads.

Comment thread examples/royco/src/RoycoHelpers.sol Outdated

constructor(address tranche_) {
tranche = tranche_;
kernel = IRoycoVaultTranche(tranche_).KERNEL();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This constructor also reads protocol state from the tranche (KERNEL and TRANCHE_TYPE). Please avoid live protocol calls during construction; pass these values explicitly or defer them to fork-aware reads.


pool = pool_;
oracle = oracle_;
baseCurrency = IAaveV3LikeOracle(oracle_).BASE_CURRENCY();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This reads oracle configuration during construction (BASE_CURRENCY, and BASE_CURRENCY_UNIT on the next line). To keep the assertion deployable in the Credible runtime, these should be constructor arguments or fork-time reads rather than live oracle calls in the constructor.

constructor(address vault_) {
require(vault_ != address(0), "SymbioticVaultBase: vault is zero");
vault = vault_;
asset = ISymbioticVaultLike(vault_).collateral();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This constructor reads vault_.collateral() from the protected vault. Please pass the collateral asset explicitly, or read it later through the fork-aware helper path, so assertion deployment does not depend on live adopter code being available in the constructor context.

@fredo
Copy link
Copy Markdown
Contributor

fredo commented May 26, 2026

Coverage/CI concern: the new protocol examples are effectively compile-only right now.

forge test --no-match-path 'test/backtesting/**' passes locally, but the new protocol folders do not appear to have meaningful protocol-specific tests. The current test pass mostly covers existing utilities and Aave-like decode/unit coverage, not the newly added Aave/Aerodrome/Cap/Curve/Denaria/Euler/Nado/Royco/Safe/Spark/Symbiotic/Tydro/Uniswap/Veda/ZeroEx protections. That means issues like missing spec registration, constructor-time protocol reads, trigger wiring mistakes, and bad-path assertion behavior can slip through while the repo still reports green tests.

Related CI gap: this PR adds per-protocol Foundry profiles in foundry.toml, but .github/workflows/solidity-test.yml only builds the assertions-book example profile. Future changes could break most of these examples without CI noticing.

Can we add a CI matrix that at least runs forge build for every new protocol profile, and add focused tests for the new examples? I would expect each protocol example to have at minimum:

  • a deployment/spec-recording smoke test where applicable
  • one passing scenario for the intended safe behavior
  • one failing/bad-path scenario proving the protection actually trips
  • coverage for constructor/dependency wiring so deployability regressions are caught

Without that, these protections are hard to evaluate beyond syntax/compilation.

Addresses PR #70 review comments from @fredo.

Constructor-time protocol reads removed. Adopter/protocol state can revert
with EXTCODESIZE = 0 in the Credible Layer's assertion-deploy runtime, so
helpers now take all derived addresses and constants as explicit args:
- CurveLlammaProtocol: borrowedToken, collateralToken, precisions
- CurveUsdProtocol: amm
- LlamaLendProtocol: controller (vault helper), borrowedToken (controller helper)
- RoycoHelpers: accountant + tranches + assets (kernel), kernel + trancheType (tranche)
- SparkLendOraclePriceGuardAssertion: baseCurrency + baseCurrencyUnit
- SymbioticVaultBaseAssertion: asset

Inheritance chains are updated to thread the new args through.

registerAssertionSpec(AssertionSpec.Reshiram) added to every concrete leaf
assertion that was missing one, including the protections flagged by the
review (Denaria, Spark, LlamaLend, Euler, Royco, Symbiotic, Veda Boring
Vault) and the other concrete leaves under examples/ for consistency.
@makemake-kbo
Copy link
Copy Markdown
Contributor Author

Addressed the constructor-read and spec-registration items from this review in 9b3049a. Summary:

Constructor-time protocol reads — pulled the live adopter reads out of every helper flagged here. Each now takes the derived addresses/constants as explicit constructor args, matching the existing ERC4626BaseAssertion pattern (the rationale comment from that file is now mirrored on each touched helper):

  • examples/curve/src/CurveLlammaProtocol.solborrowedToken, collateralToken, precisions are explicit; removed the decimals() fallback and the unused IERC20MetadataLike interface.
  • examples/curve/src/CurveUsdProtocol.solamm is explicit.
  • examples/curve/src/LlamaLendProtocol.sol — vault helper takes controller; controller helper takes borrowedToken.
  • examples/royco/src/RoycoHelpers.sol — kernel helper takes accountant + senior/junior tranches + assets; tranche helper takes kernel + tranche type.
  • examples/spark/src/SparkLendOraclePriceGuardAssertion.solbaseCurrency and baseCurrencyUnit are explicit.
  • examples/symbiotic/src/SymbioticVaultBaseAssertion.solasset is explicit.

All concrete inheritors were updated to thread the new args through.

Assertion spec registration — added registerAssertionSpec(AssertionSpec.Reshiram) to every concrete leaf assertion that was missing one. This covers everything you called out (Denaria, Spark vault/oracle/SLL, LlamaLend vault, Euler, Royco kernel/tranche, Symbiotic vault, Veda Boring Vault) plus the other concrete leaves under examples/ for consistency. The abstract bases (PerpetualBaseAssertion, ERC4626BaseAssertion, RoycoKernelHelpers, RoycoVaultTrancheHelpers, SymbioticVaultBaseAssertion, EulerEVaultHelpers, BoringVaultHelpers) intentionally do not call registerAssertionSpec since SpecRecorder.registerAssertionSpec is once-per-assertion and several of these bases have multiple concrete derivatives.

Verification — all 20 foundry profiles still build clean, and forge test --no-match-path 'test/backtesting/**' passes (64/64).

Deferred to a follow-up: the CI matrix / per-protocol tests concern. That is a meaningful change in test scope and I didn't want to bundle it with this fix. Happy to open a separate PR that wires a forge build matrix in solidity-test.yml over each new profile plus skeleton deployment/spec-recording smoke tests, if you'd like.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants