Skip to content

feat(erc4626): exit amount intent validation (ENG-3751, ENG-3752) - #33

Merged
ajag408 merged 6 commits into
mainfrom
feat/eng-3751-eng-3752-erc4626-exit-amount-validation
Aug 13, 2026
Merged

feat(erc4626): exit amount intent validation (ENG-3751, ENG-3752)#33
ajag408 merged 6 commits into
mainfrom
feat/eng-3751-eng-3752-erc4626-exit-amount-validation

Conversation

@ajag408

@ajag408 ajag408 commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Summary

  • withdraw: opt-in match of calldata assets vs args.amount within 10 wei (ENG-3752 / DFNS withdraw-on-amount path).
  • redeem: opt-in match of calldata shares vs args.shareAmount with bounded margin (ENG-3751).
  • Margin: "10" by default; decimal-gap widen when tx.to is an allocator (allocatorVaults). Kiln-set OAVs force "10" for monorepo parity. No args.feeConfigurationId for margin.
  • Method-swap / both-declared fail-closed; absent amount/shareAmount unchanged (structural only).
  • Re-export vault-registry.json with inputTokenDecimals / vaultTokenDecimals.
  • Docs: README amount-intent section for enter/exit. Bump to 1.5.0.

Test plan

  • pnpm test --testPathIgnorePatterns=tron (6089 passed)
  • Focused: amount.test.ts, erc4626.validator.test.ts, handler.test.ts, vault-registry.test.ts
  • CLI smoke: withdraw ±10/±11; redeem + shareAmount; absent args; allocator wide vs underlying tight
  • Registry: decimals on all vaults; allocators present
  • After publish: DFNS bumps @yieldxyz/shield and passes args.amount on withdraw
  • Transitional shareAmount clients: within margin / block outside
  • Omitting args: no behavior change
  • Enter deposit/approve/WRAP amount checks unchanged

Notes for reviewers

  • Opt-in only — no intent check unless args.amount or args.shareAmount is set.
  • context.feeConfiguration can classify an address for redeem margin but does not whitelist unknown OAVs yet (still need registry re-export).
  • Kiln address list is hardcoded (mirrors monorepo VAULTS_REQUIRING_INPUT_TOKEN_DECIMALS); margin force only, no amount scaling.

@raiseerco raiseerco left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

comments

const to = txTo.toLowerCase();
if (vaultInfo.allocatorVaults?.includes(to)) return true;
return (
context?.feeConfiguration?.some(

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

if I'm not wrong, context.feeConfiguration appears to let the base vault self classify as an allocator, widening redeem margin from 10 to the decimal-gap margin on 6/18 vaults...that seems to bypass the intended underlying vault margin policy 🤔

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Great catch!!!

Since context cannot whitelist an unknown OAV yet, this branch currently provides no legitimate benefit and can widen the base-vault policy if the base address appears in context. I shipped it as a preparation for the OAV hot path feature but thanks for catching this.

I’ll restrict allocator classification to the registry’s allocatorVaults for this PR and handle runtime OAV provenance alongside the whitelist hot-path.

? args.amount
: undefined;

const declaredShareAmount = isNonEmptyString(args?.shareAmount)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

args.shareAmount is ignored outside redeem, so approval/wrap/supply can still validate structurally with a share-denominated intent present, that seems to weaken the intended fail-closed method-swap behaviour

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Agreed. I’ll reject shareAmount for APPROVAL, WRAP, and SUPPLY before routing so a redeem intent cannot be replaced with those methods. I’ll retain UNWRAP as an explicit exception because WETH share exits produce a legitimate follow-up unwrap using the same action args.

@ajag408
ajag408 requested a review from raiseerco August 11, 2026 20:01

@jdomingos jdomingos left a comment

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.

Couldn't find any issue with this, looks mint! Great 👏

@ajag408
ajag408 merged commit 9bd0322 into main Aug 13, 2026
11 checks passed
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.

4 participants