clear signing #198
clear signing #198
Conversation
There was a problem hiding this comment.
Pull Request Overview
Introduces "clear signing" (EIP-7730 style) preview support in the wallet by parsing transaction calldata into a structured intent/items model and surfacing warnings in the session request UI. Also adjusts example app configuration (enables Yttrium debug mode, switches example chain to Optimism, updates stub transaction) and removes a package entry.
- Added clear signing state, parsing logic (computeClearSigningPreview) and UI section (intent, items, warnings)
- Switched example chain/network and stub transaction target/data
- Enabled yttriumDebug flag and removed Yttrium package entry from resolved dependencies
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| Package.swift | Enables yttriumDebug flag (was false) |
| SessionRequestView.swift | Adds scroll container and new clear signing intent/items/warnings UI sections |
| SessionRequestPresenter.swift | Imports YttriumUtilsWrapper; adds published clear signing properties and parsing logic with computeClearSigningPreview |
| Package.resolved | Removes Yttrium package entry from resolution file |
| SignInteractor.swift | Replaces mainnet chain (1) with Optimism (10) in proposal namespaces |
| SessionAccountPresenter.swift | Updates stub transaction recipient and calldata for demonstration |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| // Determine if Yttrium should be used in debug (local) mode | ||
| let yttriumDebug = false | ||
| let yttriumDebug = true |
There was a problem hiding this comment.
[nitpick] Hard-coding the debug flag to true risks shipping debug behavior in non-debug builds. Consider wrapping with #if DEBUG or sourcing from an environment/config so release builds keep this false.
| let displayModel = try! clearSigningFormat( | ||
| chainId: chainIdNumber, | ||
| to: to, | ||
| calldataHex: calldataHex | ||
| ) | ||
|
|
||
| // let displayModel = try! clearSigningFormat( | ||
| // chainId: 10, | ||
| // to: "0x521B4C065Bbdbe3E20B3727340730936912DfA46", | ||
| // calldataHex: "0x7c616fe60000000000000000000000000000000000000000000000000000000067741500" | ||
| // ) | ||
|
|
||
|
|
||
| clearSigningIntent = displayModel.intent | ||
| clearSigningItems = displayModel.items.map { ($0.label, $0.value) } | ||
| clearSigningWarnings = displayModel.warnings | ||
| if let raw = displayModel.raw { | ||
| clearSigningRawSelector = raw.selector | ||
| clearSigningRawArgs = raw.args |
There was a problem hiding this comment.
Using try! will crash the app if clearSigningFormat throws (e.g., malformed calldata from an external request). Replace with do/catch and gracefully fall back (e.g., populate raw selector/args only or skip intent) to avoid a denial-of-service vector.
| let displayModel = try! clearSigningFormat( | |
| chainId: chainIdNumber, | |
| to: to, | |
| calldataHex: calldataHex | |
| ) | |
| // let displayModel = try! clearSigningFormat( | |
| // chainId: 10, | |
| // to: "0x521B4C065Bbdbe3E20B3727340730936912DfA46", | |
| // calldataHex: "0x7c616fe60000000000000000000000000000000000000000000000000000000067741500" | |
| // ) | |
| clearSigningIntent = displayModel.intent | |
| clearSigningItems = displayModel.items.map { ($0.label, $0.value) } | |
| clearSigningWarnings = displayModel.warnings | |
| if let raw = displayModel.raw { | |
| clearSigningRawSelector = raw.selector | |
| clearSigningRawArgs = raw.args | |
| do { | |
| let displayModel = try clearSigningFormat( | |
| chainId: chainIdNumber, | |
| to: to, | |
| calldataHex: calldataHex | |
| ) | |
| clearSigningIntent = displayModel.intent | |
| clearSigningItems = displayModel.items.map { ($0.label, $0.value) } | |
| clearSigningWarnings = displayModel.warnings | |
| if let raw = displayModel.raw { | |
| clearSigningRawSelector = raw.selector | |
| clearSigningRawArgs = raw.args | |
| } | |
| } catch { | |
| // Graceful fallback: skip intent/items/warnings, try to extract raw selector/args if possible, or leave blank | |
| clearSigningIntent = nil | |
| clearSigningItems = [] | |
| clearSigningWarnings = [] | |
| clearSigningRawSelector = nil | |
| clearSigningRawArgs = [] |
| ForEach(0..<presenter.clearSigningWarnings.count, id: \.self) { idx in | ||
| verifyDescriptionView(imageName: "exclamationmark.circle.fill", title: "Warning", description: presenter.clearSigningWarnings[idx], color: .orange) |
There was a problem hiding this comment.
[nitpick] Using a range ForEach with indices can cause view identity issues if the array changes. Prefer ForEach(presenter.clearSigningWarnings, id: .self) { warning in ... } since the warnings are Strings (Hashable) and provide stable identity.
| ForEach(0..<presenter.clearSigningWarnings.count, id: \.self) { idx in | |
| verifyDescriptionView(imageName: "exclamationmark.circle.fill", title: "Warning", description: presenter.clearSigningWarnings[idx], color: .orange) | |
| ForEach(presenter.clearSigningWarnings, id: \.self) { warning in | |
| verifyDescriptionView(imageName: "exclamationmark.circle.fill", title: "Warning", description: warning, color: .orange) |
| ForEach(0..<presenter.clearSigningItems.count, id: \.self) { idx in | ||
| let item = presenter.clearSigningItems[idx] | ||
| VStack(alignment: .leading, spacing: 4) { | ||
| Text(item.label) | ||
| .foregroundColor(.grey50) | ||
| .font(.system(size: 12, weight: .semibold, design: .rounded)) | ||
| Text(item.value) | ||
| .foregroundColor(.grey8) | ||
| .font(.system(size: 13, weight: .semibold, design: .rounded)) | ||
| .fixedSize(horizontal: false, vertical: true) | ||
| } |
There was a problem hiding this comment.
[nitpick] Index-based ForEach can lead to unstable identities when the underlying array mutates. Consider giving items a struct with an id or using enumerated() and a stable identifier (e.g., label) to ensure predictable diffing.
|
Description
Resolves # (issue)
How Has This Been Tested?
Due Dilligence