feat: humanize Solidity contract errors from auto-generated selector map#1922
feat: humanize Solidity contract errors from auto-generated selector map#1922Tranquil-Flow wants to merge 11 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis pull request adds decoding of custom Solidity error selectors into human-readable messages. A new utility function Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Tranquil-Flow
had a problem deploying
to
development
GitHub Actions
Failure
…r display Adds @selfxyz/new-common as a dependency to packages/webview-app and app/. Applies humanizeContractError in provingUtils.ts (getFailureState + normalizeError) and ProofRequestStatusScreen.tsx at the failure reason display site. Adds a named ./src/blockchain export to new-common/package.json so the import path resolves cleanly without an explicit /index suffix. Also adds src/blockchain/contractErrors as an explicit tsup entry.
Tranquil-Flow
had a problem deploying
to
development
GitHub Actions
Failure
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 74c919bc-a41f-4d71-82c8-ce45a2b9dba4
📒 Files selected for processing (5)
contracts/error-selectors.jsonerror-selectors.jsonpackages/webview-app/src/screens/tunnel/TunnelResultScreen.tsxpackages/webview-app/src/utils/provingUtils.test.tspackages/webview-app/tests/screens/tunnel/tunnelFlowScreens.test.tsx
💤 Files with no reviewable changes (2)
- error-selectors.json
- contracts/error-selectors.json
The webview-app now imports humanizeContractError from @selfxyz/new-common, but the Vercel buildCommand did not include building that package. This caused the deployment to fail with a missing export error.
…ents normalizeError() creates a new object every render, so using it as an effect dependency caused tunnel_result_failure analytics to fire repeatedly. Use the stable message string instead.
|
@greptileai review |
Tranquil-Flow
changed the title
Greptile SummaryThis PR adds contract error humanization to both the Self Wallet app and the WebView SDK — replacing raw 4-byte hex selectors shown to users with readable strings. It introduces Key changes:
Confidence Score: 5/5Safe to merge — all decode paths are correct, fallbacks are in place, and both consumer integration points have passing test coverage. Core logic in humanizeContractError is correct for all three error types (Error(string), Panic(uint256), custom selector) with a safe pass-through for unknown input. Tests are comprehensive (12 unit tests + integration tests in provingUtils and tunnel flow). Package dependencies, build chain, and export paths are all properly configured. The two P2 observations (single-word SCREAMING name gap in formatErrorName, CWD assumption in the script) don't affect current correctness and don't block merge. new-common/src/blockchain/contractErrors.ts — minor formatErrorName edge case; contracts/scripts/findErrorSelectors.ts — CWD path assumption. Important Files Changed
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| const mapOutputFile = path.resolve( | ||
| process.cwd(), | ||
| "../new-common/src/data/error-selector-map.json", | ||
| ); | ||
| writeFileSync(mapOutputFile, JSON.stringify(selectorMap, null, 2)); | ||
| console.log(`💾 Selector map saved to ${mapOutputFile}`); |
There was a problem hiding this comment.
contracts/
process.cwd() is used as the anchor, which works correctly when yarn find:error is run from inside the contracts/ workspace as documented. However, if someone runs the script from the monorepo root (e.g., node contracts/scripts/findErrorSelectors.ts), process.cwd() resolves to the repo root and ../new-common/src/data/error-selector-map.json points outside the repo.
Since this is a developer tool rather than production code the risk is low, but a __dirname-relative path would be more robust:
| const mapOutputFile = path.resolve( | |
| process.cwd(), | |
| "../new-common/src/data/error-selector-map.json", | |
| ); | |
| writeFileSync(mapOutputFile, JSON.stringify(selectorMap, null, 2)); | |
| console.log(`💾 Selector map saved to ${mapOutputFile}`); | |
| const mapOutputFile = path.resolve( | |
| new URL('.', import.meta.url).pathname, | |
| '../../..', | |
| 'new-common/src/data/error-selector-map.json', | |
| ); |
Alternatively, add an explicit guard at the top of the script verifying that the CWD ends with /contracts.
Resolves conflicts from 53 commits of dev divergence. Take dev's findErrorSelectors.ts which adds collision detection, a check mode, and repo-root path resolution. Regenerate error-selectors.json and new-common/src/data/error-selector-map.json from the current contracts so the check-error-selectors workflow passes. Restore new-common/src/blockchain/contractErrors.ts and keep contractErrors.test.ts. These were removed from dev as a temporary cleanup ahead of this PR landing the humanizer. Remove the stale contracts/error-selectors.json. Merge TunnelResultScreen with both the normalizeError humanization from this branch and the isDemoMode onContinue branch from dev. Extend the TunnelResultState source union to include kyc. Combine vercel.json so the build chain still includes new-common while preserving dev's ignoreCommand and adding new-common to the ignored paths. Update stale tunnelFlowScreens close-navigation tests to assert lifecycle.setResult plus dismiss semantics and keep the humanizer assertion. Update the receipt-from-success-context test to assert close-only controls.
Tranquil-Flow
temporarily deployed
to
development
GitHub Actions
Inactive
webview-app now imports humanizeContractError from @selfxyz/new-common/src/blockchain, so the dependency must be built before typecheck and build. The webview-app-ci workflow was building common, mobile-sdk-alpha, and webview-bridge but not new-common, causing TS2307 Cannot find module errors in build and types jobs. Add yarn workspace @selfxyz/new-common build as the first dep-build step in the build, lint, and types jobs.
Tranquil-Flow
temporarily deployed
to
development
GitHub Actions
Inactive
yarn types at repo root runs each workspace's types script in topological dev order, but those scripts only run tsc noEmit. webview-app now consumes @selfxyz/new-common at type-resolution time, so new-common needs a prior build to emit its dist/esm/src/blockchain declarations. Add an explicit yarn workspace @selfxyz/new-common build before yarn types in the type-check job. Bump the Android and iOS bundle thresholds from 48 to 49 MB. Bundling humanizeContractError and the 140-entry error-selector-map.json into the mobile-app bundle adds roughly 86 KB, which pushed the Android bundle past the previous ceiling.
Tranquil-Flow
temporarily deployed
to
development
GitHub Actions
Inactive
1 participant
Closes SELF-1369
Summary
contracts/scripts/findErrorSelectors.tsto emit a deduplicatedselector → namemap tonew-common/src/data/error-selector-map.jsonalongside the existing detailed outputhumanizeContractError(raw)tonew-common/src/blockchain/— decodesError(string),Panic(uint256), and custom contract errors into readable stringshumanizeContractErrorinto both error display paths:provingUtils.tsin webview-app andProofRequestStatusScreen.tsxin the Self Wallet appcontracts/error-selectors.json(transient script output)@selfxyz/new-commonas a dependency toapp/andpackages/webview-app/How it works
The selector map stays in sync with contracts automatically — run
yarn find:errorincontracts/after adding new custom errors.humanizeContractErroris exported from@selfxyz/new-common/src/blockchainand applied at both failure reason display sites so users see readable messages instead of raw hex selectors.Integration
Both consuming packages are updated in this PR:
provingUtils.ts:getFailureStateandnormalizeErrornow pass raw error strings throughhumanizeContractErrorbefore surfacing them to the UIProofRequestStatusScreen.tsx: the failure reason rendered to the user is now decoded viahumanizeContractErrorTest plan
cd new-common && yarn test src/blockchain/contractErrors.test.ts— 12 tests passcd new-common && yarn types— no errorscd packages/webview-app && yarn types— no errorsyarn workspace @selfxyz/mobile-app types— no errorscd contracts && yarn find:error— regenerateserror-selector-map.jsoncorrectlySupersedes
Closes #1911
Summary by CodeRabbit
New Features
Tests