fix(S3): validate constructor critical parameters

[liobrasil]

Summary

Fixes #34

Adds validation for critical constructor/config parameters in FlowYieldVaultsRequests to avoid misconfiguration at deploy time and prevent zero-minimum supported tokens.

Key points:

• Constructor now reverts if coaAddress == address(0)
• Introduced InvalidMinimumBalance for invalid supported-token minimums
• Token configuration now enforces minimumBalance > 0 when isSupported == true
• Constructor token setup and admin token updates share one internal validation path (_setTokenConfig)

Changes:

• Added constructor guard for zero COA address
• Added internal _setTokenConfig(...) helper with validation
• Updated constructor + setTokenConfig(...) to use shared helper
• Added tests for constructor zero-COA revert and zero-minimum supported-token revert

Test plan

• FOUNDRY_OFFLINE=true forge test --match-contract FlowYieldVaultsRequestsTest

[claude]

PR Review: fix(S3): validate constructor critical parameters

Overall: This is a clean, well-scoped improvement. The changes are correct, follow existing patterns in the codebase (reusing InvalidCOAAddress, which already guards setAuthorizedCOA), and the tests cover the new validation paths. A few minor observations below.

---

Strengths

• DRY: Extracting the duplicate struct assignment into _setTokenConfig removes two identical code blocks and ensures constructor and admin paths share a single validation path — exactly the right pattern.
• Named constant: DEFAULT_MINIMUM_BALANCE = 1 ether removes the magic 1 ether literals. Good.
• Guard reuse: Reusing the pre-existing InvalidCOAAddress error in the constructor is consistent with setAuthorizedCOA.
• Validation condition: if (isSupported && minimumBalance == 0) correctly allows minimumBalance = 0 when disabling a token (isSupported = false), which is the right semantics.

---

Issues / Observations

1. Missing tokenAddress zero-address check in _setTokenConfig (pre-existing gap, but now is the natural moment to fix)

_setTokenConfig doesn't validate tokenAddress != address(0). An admin could call setTokenConfig(address(0), true, 1 ether, false), inadvertently configuring the zero address as a supported token. The constructor is safe because it only calls this with NATIVE_FLOW and WFLOW, but the external setTokenConfig path is open. Consider adding:

if (tokenAddress == address(0)) revert InvalidTokenAddress(); // or reuse a suitable existing error

2. setTokenConfig emits event but constructor doesn't — minor inconsistency

setTokenConfig calls _setTokenConfig then emits TokenConfigured. The constructor also calls _setTokenConfig for NATIVE_FLOW (and optionally WFLOW) but emits nothing. This was the same before this PR, but since _setTokenConfig is now a shared path it's worth noting: the constructor-time token configuration is silent on-chain. For a deploy-and-monitor workflow this is fine, but indexers/subgraphs that rely on TokenConfigured events won't see the initial state.

3. Redundant > 0 check in _validateDeposit (minor)

_validateDeposit at line ~1475 guards with if (config.minimumBalance > 0 && ...). With this PR's invariant that all supported tokens must have minimumBalance > 0, the > 0 half of that condition is now always true for supported tokens. Not a bug — it's defensive — but worth noting as dead logic.

4. Missing test coverage

• isSupported = false, minimumBalance = 0 (valid disable path): The new tests cover the revert paths, but there's no test asserting that setTokenConfig(token, false, 0, false) succeeds (the boundary case the condition is designed to allow).
• Constructor default state: No test verifies that after construction allowedTokens[NATIVE_FLOW].minimumBalance == DEFAULT_MINIMUM_BALANCE. Given the constant was introduced specifically for this, a snapshot test would lock in that behavior.
• Constructor with wflowAddress == address(0): No test confirms WFLOW is not configured when zero address is passed.

---

Summary

The core changes are correct and well-structured. Items 1 (zero-address token guard) and 4 (missing test cases) are the most actionable. Everything else is informational. Happy to approve once the token-address guard question is addressed or explicitly dismissed as out of scope.