fix(wallet): local storage managed wallet bug + support multiple managed#2307
fix(wallet): local storage managed wallet bug + support multiple managed#2307
Conversation
WalkthroughPer-network, per-user managed-wallet persistence introduced: managed wallets moved to network-scoped maps in localStorage; wallet utilities updated (signatures changed) to read/write/merge managed and custodial wallets with error handling; a hook effect in Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| @@ -394,7 +394,6 @@ | |||
| "resolved": "https://registry.npmjs.org/@opentelemetry/core/-/core-2.0.0.tgz", | |||
There was a problem hiding this comment.
🔄 Carefully review the package-lock.json diff
Resolve the comment if everything is ok
+ node_modules/git-semver-tags/node_modules/conventional-commits-filter 5.0.0
+ node_modules/git-semver-tags/node_modules/conventional-commits-parser 6.2.1 
There was a problem hiding this comment.
question: @baktun14 is this needed or sent by mistake? it doesn't seem related
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2307 +/- ##
==========================================
+ Coverage 47.60% 48.01% +0.40%
==========================================
Files 1036 1038 +2
Lines 29357 29667 +310
Branches 7632 7617 -15
==========================================
+ Hits 13976 14244 +268
+ Misses 15000 14950 -50
- Partials 381 473 +92
🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
apps/deploy-web/src/utils/walletUtils.spec.ts (2)
243-250: Consider testingundefinedexplicitly.The function signature allows
undefinedforuserId. Line 246 testsundefinedimplicitly by calling with no args, but line 247 tests empty string. Consider making theundefinedcase explicit for clarity.it("returns undefined when userId is not provided", () => { const { cleanup } = setup(); - expect(getStorageManagedWallet()).toBeUndefined(); + expect(getStorageManagedWallet(undefined)).toBeUndefined(); expect(getStorageManagedWallet("")).toBeUndefined(); cleanup(); });
434-453: Test assertion could be stricter.The test verifies that results are equal but doesn't confirm that storage wasn't written twice. Consider verifying that
setItemwas called only once (on the first call).it("returns same object if no changes (optimization)", () => { - const { cleanup } = setup(); + const { cleanup, mockLocalStorage } = setup(); const wallet = { address: WALLET_ADDRESS_1, cert: "cert1", certKey: "key1", userId: USER_ID_1, creditAmount: 100, isTrialing: false, selected: true }; const result1 = updateStorageManagedWallet(wallet); + const callCountAfterFirst = mockLocalStorage.setItem.mock.calls.length; const result2 = updateStorageManagedWallet(wallet); expect(result1).toEqual(result2); + expect(mockLocalStorage.setItem.mock.calls.length).toBe(callCountAfterFirst); cleanup(); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (3)
apps/deploy-web/src/hooks/useManagedWallet.ts(3 hunks)apps/deploy-web/src/utils/walletUtils.spec.ts(1 hunks)apps/deploy-web/src/utils/walletUtils.ts(4 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.{ts,tsx,js}: Never use typeanyor cast to typeany. Always define the proper TypeScript types.
Never use deprecated methods from libraries.
Don't add unnecessary comments to the code.
Files:
apps/deploy-web/src/hooks/useManagedWallet.tsapps/deploy-web/src/utils/walletUtils.tsapps/deploy-web/src/utils/walletUtils.spec.ts
**/*.spec.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/no-jest-mock.mdc)
Don't use
jest.mock()in test files. Instead, usejest-mock-extendedto create mocks and pass mocks as dependencies to the service under testUse
setupfunction instead ofbeforeEachin test files. Thesetupfunction must be at the bottom of the rootdescribeblock, should create an object under test and return it, accept a single parameter with inline type definition, avoid shared state, and not have a specified return type.
**/*.spec.{ts,tsx}: Use<Subject>.namein the root describe suite description instead of hardcoded class/service name strings to enable automated refactoring tools to find all references
Use either a method name or a condition starting with 'when' for nested suite descriptions in tests
Use present simple, 3rd person singular for test descriptions without prepending 'should'
Files:
apps/deploy-web/src/utils/walletUtils.spec.ts
🧠 Learnings (2)
📓 Common learnings
Learnt from: jzsfkzm
Repo: akash-network/console PR: 2039
File: apps/deploy-web/tests/ui/change-wallets.spec.ts:4-10
Timestamp: 2025-10-15T16:39:55.348Z
Learning: In the Akash Console E2E tests using the context-with-extension fixture, the first wallet is automatically created during fixture setup via `importWalletToLeap` in `apps/deploy-web/tests/ui/fixture/wallet-setup.ts`, so tests that call `frontPage.createWallet()` are creating a second wallet to test wallet switching functionality.
📚 Learning: 2025-10-15T16:39:55.348Z
Learnt from: jzsfkzm
Repo: akash-network/console PR: 2039
File: apps/deploy-web/tests/ui/change-wallets.spec.ts:4-10
Timestamp: 2025-10-15T16:39:55.348Z
Learning: In the Akash Console E2E tests using the context-with-extension fixture, the first wallet is automatically created during fixture setup via `importWalletToLeap` in `apps/deploy-web/tests/ui/fixture/wallet-setup.ts`, so tests that call `frontPage.createWallet()` are creating a second wallet to test wallet switching functionality.
Applied to files:
apps/deploy-web/src/utils/walletUtils.spec.ts
🧬 Code graph analysis (2)
apps/deploy-web/src/utils/walletUtils.ts (1)
packages/network-store/src/network.store.ts (1)
selectedNetworkId(56-58)
apps/deploy-web/src/utils/walletUtils.spec.ts (1)
apps/deploy-web/src/utils/walletUtils.ts (10)
getStorageWallets(114-150)LocalWallet(27-27)getSelectedStorageWallet(29-33)getStorageManagedWallet(35-58)updateStorageManagedWallet(60-86)updateStorageWallets(164-183)updateWallet(152-162)deleteWalletFromStorage(185-203)deleteManagedWalletFromStorage(88-112)ensureUserManagedWalletOwnership(209-221)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (16)
- GitHub Check: validate (apps/notifications) / validate-unsafe
- GitHub Check: validate (packages) / validate-unsafe
- GitHub Check: validate / validate-app
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: test-build
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (8)
apps/deploy-web/src/hooks/useManagedWallet.ts (2)
10-10: LGTM!The import changes align with the refactored storage model—removing the unused
deleteManagedWalletFromStorageand adding the per-user storage utilities.
40-50: Effect dependencies include object reference that may trigger unnecessary runs.The
walletdependency is a memoized object (line 23), but its identity changes wheneverqueriedorcreatedchanges. SinceisEqualchecks happen insideupdateStorageManagedWallet, this should be safe, but worth noting.The removal of the deletion branch that ran when
!wallet && wasFetchedis the key fix—this prevents accidental wallet deletion on disconnect/logout scenarios.apps/deploy-web/src/utils/walletUtils.ts (3)
35-58: LGTM!The per-user, per-network managed wallet retrieval is well-implemented with proper null checks and graceful JSON error handling.
122-147: LGTM!Good defensive coding with try/catch around JSON parsing and proper fallback to original wallets on error.
209-220: LGTM!The ownership enforcement correctly selects the managed wallet and deselects custodial wallets for the same network.
apps/deploy-web/src/utils/walletUtils.spec.ts (3)
18-26: LGTM!Good test organization with meaningful constants for test data. The root describe uses a module name string which is acceptable for utility modules (as opposed to classes where
.namewould be preferred).
943-1030: Excellent regression test coverage.These tests directly address the critical bug being fixed—ensuring managed wallets persist across disconnects and multiple users don't interfere with each other's data.
1144-1217: LGTM!The
setupfunction follows the coding guidelines: placed at the bottom of the describe block, creates isolated state per test via theMap, and returns cleanup function. Good use ofjest-mock-extendedfor the localStorage mock with proper proxy handling forObject.keys()enumeration.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| localStorage.setItem(managedWalletsKey, JSON.stringify(existingMap)); | ||
| } | ||
|
|
||
| localStorage.setItem(`${selectedNetworkId}/wallets`, JSON.stringify(custodialWallets)); |
Check failure
Code scanning / CodeQL
Clear text storage of sensitive information High
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
apps/deploy-web/src/utils/walletUtils.ts (3)
82-84: Missing try/catch around JSON.parse.As flagged in previous reviews, if
walletsMapStrcontains malformed JSON, this will throw an unhandled exception, unlike the read path ingetStorageManagedWalletwhich gracefully handles corrupt data.
101-112: Missing try/catch around JSON.parse.As flagged in previous reviews, malformed JSON will cause an unhandled exception. This should be wrapped in a try/catch block consistent with the pattern used in
getStorageManagedWallet.
174-183: Missing try/catch around JSON.parse.As flagged in previous reviews, this should handle malformed JSON gracefully consistent with the pattern used in
getStorageManagedWallet.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/deploy-web/src/utils/walletUtils.ts(4 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.{ts,tsx,js}: Never use typeanyor cast to typeany. Always define the proper TypeScript types.
Never use deprecated methods from libraries.
Don't add unnecessary comments to the code.
Files:
apps/deploy-web/src/utils/walletUtils.ts
🧠 Learnings (2)
📓 Common learnings
Learnt from: jzsfkzm
Repo: akash-network/console PR: 2039
File: apps/deploy-web/tests/ui/change-wallets.spec.ts:4-10
Timestamp: 2025-10-15T16:39:55.348Z
Learning: In the Akash Console E2E tests using the context-with-extension fixture, the first wallet is automatically created during fixture setup via `importWalletToLeap` in `apps/deploy-web/tests/ui/fixture/wallet-setup.ts`, so tests that call `frontPage.createWallet()` are creating a second wallet to test wallet switching functionality.
📚 Learning: 2025-10-31T11:26:42.138Z
Learnt from: stalniy
Repo: akash-network/console PR: 2138
File: apps/api/src/billing/lib/batch-signing-client/batch-signing-client.service.ts:379-382
Timestamp: 2025-10-31T11:26:42.138Z
Learning: In TypeScript/JavaScript, the pattern of checking a cached value and then performing an async operation to fetch it without proper synchronization is race condition unsafe:
```typescript
private async getAddress() {
if (!this.address) {
this.address = await this.wallet.getFirstAddress();
}
return this.address;
}
```
Multiple concurrent calls can all pass the `if (!this.address)` check before any of them sets the value, leading to duplicate async operations. This should be flagged as a race condition. Proper synchronization (mutex, atomic promise caching, or guaranteed single-threaded execution) is required.
Applied to files:
apps/deploy-web/src/utils/walletUtils.ts
🧬 Code graph analysis (1)
apps/deploy-web/src/utils/walletUtils.ts (4)
apps/stats-web/src/config/env-config.schema.ts (1)
networkId(4-4)apps/notifications/src/interfaces/rest/services/auth/auth.service.ts (1)
userId(18-25)packages/network-store/src/network.store.ts (1)
selectedNetworkId(56-58)apps/stats-web/src/store/network.store.ts (1)
networkStore(6-10)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (14)
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: validate (apps/deploy-web) / validate-unsafe
- GitHub Check: validate (apps/provider-console) / validate-unsafe
- GitHub Check: validate (apps/api) / validate-unsafe
- GitHub Check: test-build
- GitHub Check: test-build
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: test-build
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (3)
apps/deploy-web/src/utils/walletUtils.ts (3)
29-31: LGTM!Clean helper function for generating per-network storage keys.
39-62: LGTM!The function correctly implements defensive guards (userId presence, window availability) and gracefully handles malformed JSON with try/catch.
126-153: LGTM!The function correctly merges managed and custodial wallets with proper error handling. The logic ensures managed wallet selection takes precedence and gracefully falls back to custodial wallets on parse errors.
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/deploy-web/src/utils/walletUtils.ts(5 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.{ts,tsx,js}: Never use typeanyor cast to typeany. Always define the proper TypeScript types.
Never use deprecated methods from libraries.
Don't add unnecessary comments to the code.
Files:
apps/deploy-web/src/utils/walletUtils.ts
🧠 Learnings (3)
📓 Common learnings
Learnt from: jzsfkzm
Repo: akash-network/console PR: 2039
File: apps/deploy-web/tests/ui/change-wallets.spec.ts:4-10
Timestamp: 2025-10-15T16:39:55.348Z
Learning: In the Akash Console E2E tests using the context-with-extension fixture, the first wallet is automatically created during fixture setup via `importWalletToLeap` in `apps/deploy-web/tests/ui/fixture/wallet-setup.ts`, so tests that call `frontPage.createWallet()` are creating a second wallet to test wallet switching functionality.
📚 Learning: 2025-10-31T11:26:42.138Z
Learnt from: stalniy
Repo: akash-network/console PR: 2138
File: apps/api/src/billing/lib/batch-signing-client/batch-signing-client.service.ts:379-382
Timestamp: 2025-10-31T11:26:42.138Z
Learning: In TypeScript/JavaScript, the pattern of checking a cached value and then performing an async operation to fetch it without proper synchronization is race condition unsafe:
```typescript
private async getAddress() {
if (!this.address) {
this.address = await this.wallet.getFirstAddress();
}
return this.address;
}
```
Multiple concurrent calls can all pass the `if (!this.address)` check before any of them sets the value, leading to duplicate async operations. This should be flagged as a race condition. Proper synchronization (mutex, atomic promise caching, or guaranteed single-threaded execution) is required.
Applied to files:
apps/deploy-web/src/utils/walletUtils.ts
📚 Learning: 2025-10-15T16:39:55.348Z
Learnt from: jzsfkzm
Repo: akash-network/console PR: 2039
File: apps/deploy-web/tests/ui/change-wallets.spec.ts:4-10
Timestamp: 2025-10-15T16:39:55.348Z
Learning: In the Akash Console E2E tests using the context-with-extension fixture, the first wallet is automatically created during fixture setup via `importWalletToLeap` in `apps/deploy-web/tests/ui/fixture/wallet-setup.ts`, so tests that call `frontPage.createWallet()` are creating a second wallet to test wallet switching functionality.
Applied to files:
apps/deploy-web/src/utils/walletUtils.ts
🧬 Code graph analysis (1)
apps/deploy-web/src/utils/walletUtils.ts (2)
packages/logging/src/services/logger/logger.service.ts (1)
error(141-143)apps/deploy-web/src/services/app-di-container/browser-di-container.ts (1)
services(24-59)
🪛 GitHub Check: CodeQL
apps/deploy-web/src/utils/walletUtils.ts
[failure] 220-220: Clear text storage of sensitive information
This stores sensitive data returned by an access to localCert as clear text.
This stores sensitive data returned by an access to localCert as clear text.
🪛 GitHub Check: validate / validate-app
apps/deploy-web/src/utils/walletUtils.ts
[failure] 5-5:
Dependency cycle detected
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: validate (apps/deploy-web) / validate-unsafe
- GitHub Check: test-build
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (2)
apps/deploy-web/src/utils/walletUtils.ts (2)
220-220: Acknowledge: Sensitive data stored in clear text (CodeQL finding).CodeQL flagged that certificate data (
cert,certKey) is stored in localStorage as clear text. While localStorage is inherently unencrypted and this may be an accepted trade-off for browser-based wallet functionality, consider whether:
- The certificate data can be encrypted before storage
- A more secure storage mechanism (e.g., IndexedDB with encryption) would be appropriate
- The security implications are documented and accepted
If this is accepted risk, consider adding a comment or documentation noting the security trade-off.
247-261: LGTM!The refactored implementation now properly handles managed wallet selection by iterating over all wallets and ensuring only the matching managed wallet has
selected: true. This addresses the previous concern about multiple managed wallets potentially havingselected: true.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/deploy-web/src/utils/walletUtils.ts (1)
9-10: Consider using the DI container for service instances.Creating new service instances directly works but means this module has separate instances of ErrorHandlerService rather than sharing the application's singleton. If ErrorHandlerService has any configuration or shared state, this could lead to inconsistencies.
If the circular dependency with
browser-di-containercan be resolved (e.g., by lazy import or moving the error reporting to a callback pattern), prefer retrieving these services from the container.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/deploy-web/src/utils/walletUtils.ts(5 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/general.mdc)
**/*.{ts,tsx,js}: Never use typeanyor cast to typeany. Always define the proper TypeScript types.
Never use deprecated methods from libraries.
Don't add unnecessary comments to the code.
Files:
apps/deploy-web/src/utils/walletUtils.ts
🧠 Learnings (3)
📓 Common learnings
Learnt from: jzsfkzm
Repo: akash-network/console PR: 2039
File: apps/deploy-web/tests/ui/change-wallets.spec.ts:4-10
Timestamp: 2025-10-15T16:39:55.348Z
Learning: In the Akash Console E2E tests using the context-with-extension fixture, the first wallet is automatically created during fixture setup via `importWalletToLeap` in `apps/deploy-web/tests/ui/fixture/wallet-setup.ts`, so tests that call `frontPage.createWallet()` are creating a second wallet to test wallet switching functionality.
📚 Learning: 2025-10-31T11:26:42.138Z
Learnt from: stalniy
Repo: akash-network/console PR: 2138
File: apps/api/src/billing/lib/batch-signing-client/batch-signing-client.service.ts:379-382
Timestamp: 2025-10-31T11:26:42.138Z
Learning: In TypeScript/JavaScript, the pattern of checking a cached value and then performing an async operation to fetch it without proper synchronization is race condition unsafe:
```typescript
private async getAddress() {
if (!this.address) {
this.address = await this.wallet.getFirstAddress();
}
return this.address;
}
```
Multiple concurrent calls can all pass the `if (!this.address)` check before any of them sets the value, leading to duplicate async operations. This should be flagged as a race condition. Proper synchronization (mutex, atomic promise caching, or guaranteed single-threaded execution) is required.
Applied to files:
apps/deploy-web/src/utils/walletUtils.ts
📚 Learning: 2025-10-15T16:39:55.348Z
Learnt from: jzsfkzm
Repo: akash-network/console PR: 2039
File: apps/deploy-web/tests/ui/change-wallets.spec.ts:4-10
Timestamp: 2025-10-15T16:39:55.348Z
Learning: In the Akash Console E2E tests using the context-with-extension fixture, the first wallet is automatically created during fixture setup via `importWalletToLeap` in `apps/deploy-web/tests/ui/fixture/wallet-setup.ts`, so tests that call `frontPage.createWallet()` are creating a second wallet to test wallet switching functionality.
Applied to files:
apps/deploy-web/src/utils/walletUtils.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (12)
- GitHub Check: validate (apps/deploy-web) / validate-unsafe
- GitHub Check: test-build
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: validate / validate-app
- GitHub Check: test-build
- GitHub Check: test-build
- GitHub Check: test-build
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (8)
apps/deploy-web/src/utils/walletUtils.ts (8)
44-70: LGTM!The function correctly guards against missing parameters and SSR execution, handles malformed localStorage data gracefully with error reporting, and implements the new per-network lookup pattern.
72-111: LGTM!The function correctly implements the per-network map update pattern with proper error handling, graceful fallbacks for malformed data, and preserves the selected state logic from the previous wallet.
113-148: LGTM!The deletion logic correctly handles the per-network map, gracefully handles parse errors by removing the corrupted key, and properly cleans up both the map entry and the underlying wallet data.
186-191: LGTM!The selected managed wallet handling correctly ensures only one wallet is selected at a time, deselecting all others when a managed wallet is marked as selected.
158-170: LGTM!Custodial wallets are now properly protected with try/catch around JSON.parse, reporting errors and gracefully falling back to an empty array on malformed data.
220-252: LGTM!The function correctly splits managed and custodial wallets, updates each storage location separately with proper error handling, and gracefully handles malformed data in the managed wallets map.
278-293: LGTM!This function correctly addresses the previous concern about multiple managed wallets being selected simultaneously. The logic ensures only the managed wallet matching the userId is selected, with all other wallets (both managed and custodial) deselected.
180-181: The filter condition on line 181 is correct and intentional — it's not a bug. The test case "removes duplicate managed wallets from old storage" (lines 107-132 of the spec file) explicitly validates this behavior.The filter
!w.isManaged || !managedWalletAddresses.has(w.address)correctly handles migration from the legacy storage format where managed wallets were stored alongside custodial wallets. When managed wallets are moved to the new${networkId}/managed-walletsstorage, the filter prevents duplicates by removing managed wallets that now exist in the new location.The suggested simplification to
!w.isManagedwould break this deduplication logic and reintroduce duplicates during migration.
2 participants
closes #2294
Summary by CodeRabbit
New Features
Bug Fixes
Tests
✏️ Tip: You can customize this high-level summary in your review settings.