# `depositFromNotional` function is payable, which means that it should accept Ether, but in reality will revert 100% when msg.value > 0

Payable depositFromNotional delegatecalls into non-payable executeTrade, so any ETH deposit reverts, breaking ETH-pool vault deposits entirely.

- Payout: Contest reward
- Severity: medium
- Program: Notional
- Credited: AuditorPraise
- Disclosed: 2023-10-09
- Write-up quality: 6/10
- Original: https://github.com/sherlock-audit/2023-10-notional-judging/issues/51
- Library page: https://bugbountylibrary.com/r/sherlock%3A2023-10-notional%3AM-5
- Tags: domain:web3 impact:dos kind:contest platform:sherlock sev:medium tech:evm tech:solidity technique:source-review vuln:dos

**Target:** BaseStrategyVault.depositFromNotional / TradingModule.executeTrade (Solidity vaults)
**Root cause:** depositFromNotional is payable and forwards msg.value through a delegatecall to TradingModule.executeTrade, which lacks the payable keyword; delegatecall preserves msg.value, so the non-payable callee reverts whenever msg.value > 0.
**Steps:**
1. Call `vault.depositFromNotional{value: depositAmount}(account, depositAmount, maturity, data)` with msg.value > 0 (as Notional does for ETH pools).
2. depositFromNotional -> _depositFromNotional -> _executeTrade -> TradeHandler delegatecalls TradingModule.executeTrade.
3. executeTrade is non-payable; delegatecall carries msg.value, so EVM reverts.
4. Observe TradeFailed() in trace at executeTrade delegatecall.
**How found:** Reading that depositFromNotional is marked payable but the delegatecalled executeTrade is not; reasoned from EVM delegatecall semantics (msg.value preserved) that any ETH deposit must revert, then confirmed with a modified acceptance test passing value instead of dealing ETH to the vault.
**Insight:** When auditing payable entrypoints that delegatecall into shared implementation libraries, check whether the target function is payable — delegatecall forwards msg.value unconditionally, so a non-payable callee bricks the ETH path. Also verify tests actually pass msg.value rather than pre-funding the contract with deal().