fix(wallet): lock the ledger canister id once an asset has one - #667
Open
MRmarioruci wants to merge 2 commits into
Open
MRmarioruci wants to merge 2 commits into
MRmarioruci wants to merge 2 commits into
Conversation
The station rejects an edit that repoints or drops a ledger canister id that is already set, but the asset form still offered it as a free text field, so the only feedback was a rejected request. The field is now read only when the asset already has a ledger, with a hint explaining that a different ledger needs a new asset. Creating an asset and filling in a missing ledger id are unaffected, and the index canister id stays editable.
|
✅ No security or compliance issues detected. Reviewed everything up to 2a055c0. Security OverviewDetected Code Changes
|
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The production edit dialog does not supply the stored metadata needed to activate the ledger lock.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Aligns the wallet UI with the station-side ledger immutability guard from #648.
Changes:
- Makes existing ledger IDs read-only while leaving index IDs editable.
- Adds localized guidance and component tests.
File summaries
| File | Description |
|---|---|
| apps/wallet/src/locales/pt.locale.ts | Adds Portuguese guidance. |
| apps/wallet/src/locales/fr.locale.ts | Adds French guidance. |
| apps/wallet/src/locales/en.locale.ts | Adds English guidance. |
| apps/wallet/src/components/assets/standards/InternetComputerNativeStandardForm.vue | Implements the ledger lock and hint. |
| apps/wallet/src/components/assets/standards/InternetComputerNativeStandardForm.spec.ts | Tests ledger field editability. |
Review details
Suppressed comments (1)
apps/wallet/src/components/assets/standards/InternetComputerNativeStandardForm.spec.ts:49
- This setup omits
currentMetadata, so it exercises the creation path rather than the stated existing-asset-without-a-ledger case. PopulatecurrentMetadatawith another metadata key (and mirror it inmodelValue) to prove an existing asset remains editable when only the ledger key is absent.
props: {
modelValue: [],
readonly: false,
},
- Files reviewed: 5/5 changed files
- Comments generated: 1
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
The lock derived from currentMetadata, which the dialog never passes, so it only ever engaged in the request review that is already read only. The dialog now snapshots whether the loaded asset has a ledger and passes that down. Taking a snapshot rather than reading the editable model matters, since the model gains a ledger id as soon as the first character is typed, which would lock the field mid entry on an asset that does not have one.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Change
An asset's
ledger_canister_ididentifies the token that asset represents, and the station rejects an edit that repoints or drops it once set. The asset form still rendered it as a free text field, so the only feedback a user got was a rejected request.The field is now read only when the asset already has a ledger, with a persistent hint saying a different ledger needs a new asset. Two cases are deliberately unaffected: creating an asset, and filling in a ledger id on an asset that does not have one yet, which the station still allows. The index canister id stays editable.
Changes
InternetComputerNativeStandardFormderivesisLedgerIdLockedfrom thecurrentMetadataprop it already receives, so the lock keys off whether the stored asset has a ledger rather than on form mode. That drives the field'sreadonly, its variant, and the hint. Three locale files gain the hint string.Tests
vitest run src/components/assetspasses (11). Lint, typecheck and prettier clean.Note
Pairs with #648