Contracts Review Feedback

[joshuahannan]

Creating an issue with my preliminary feedback about the Cadence contract. It still isn't totally clear to me how it all fits together so I'm hoping after documenting it some more, then I'll understand it better and can complete my review.

Some of these comments are general comments that will apply to a lot of different places in the contract, like something that applies to all error messages or all events. I don't have the time to make suggestions for each individual instance of these, so I trust y'all to be thorough about all the places where my comment might apply.

• We should make all complex fields like arrays, dictionaries, structs, and resources access(contract) by default and use getters for them for external calls. Just a best practice that we should be encouraging.

• Please write some more comments in functions to describe what each section of functions is accomplishing. Also provide comments for each function that describes what they do and the parameters and returns.

• I’m confused by what needsStartProcessing, startProcessing, and CompleteProcessing are all for. Needs more comments

• Events should have as much data as possible. For example, in WorkerInitialized, we could also include the addresses of all the other capabilities if they can be different addresses, and if they are all supposed to be the same address, we should change the name of the parameter to something more general than coa address. Double check all the other events to make sure they include all the important data

• In error messages, please include relevant metadata. For example, in processRequestSafely() when the status is incorrect, include the status that was passed to show what was wrong. Example: "Request must be in PENDING (0) status but got (request.status.rawValue)”

• I assume we don’t want to revert the entire transaction if something fails in processing a single request in processRequestSafely, right? Should those pre-conditions be conditionals instead the just emit an error event if they fail? Same with all the other functions that panic or fail if something goes wrong. Can we just emit an event and move to the next one?

• Can getYieldVaultManagerRef be view?

[liobrasil]

Hi Joshua,

Thanks for the detailed review feedback on the Cadence contracts. I've implemented most of your suggestions over the past few weeks.

One item remains: handling panics in processRequestSafely. After analyzing the panic points, I've categorized them based on whether they should cause transaction reverts or fail gracefully.

---

✅ Already Fixed: Pre-condition Validation

Converted the pre-conditions in processRequestSafely to conditional checks that emit RequestFailed events and return false instead of reverting the entire batch. These validations were redundant since Solidity already enforces them, but keeping them as defensive checks prevents edge cases from blocking batch processing.

---

🛑 Keep as Panics: Systemic Failures

These indicate the Worker is fundamentally broken and should stop all processing:

• Capability failures: getCOARef(), getYieldVaultManagerRef(), getBetaReference(), feeProviderCap.borrow()

Rationale: If Worker capabilities are invalid, no requests should be processed.

---

⚠️ Should Maybe Fix: Request-Specific Failures

These can occur during normal operation and currently block entire batches:

1. Token Not Onboarded to Bridge

let vaultType = FlowEVMBridgeConfig.getTypeAssociated(with: tokenAddress)
    ?? panic("ERC20 token not onboarded")

Proposed fix: Create bridgeERC20FromEVMSafely() that returns nil instead of panicking, allowing individual request to fail gracefully.

2. ERC20 Transfer Failures

if transferResult.status != EVM.Status.successful {
    panic("ERC20 transfer to recipient failed")
}

Proposed fix: Need to think about this more deeply.

🤖 Automated Reconciliation (TBD)

FlowYieldVaults Operations

Operations like depositToYieldVault(), withdrawFromYieldVault(), and closeYieldVault() can panic for several reasons.

Proposed fix: No code changes planned. @nvdtf and I are working on automated recovery for stuck requests to handle these edge cases. We're already performing all necessary validation checks before forwarding calls to FYV but sometime an unexpected panic can happen.