Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/dull-baths-poke.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@ledgerhq/device-signer-kit-solana": patch
---

Lock in substructure framing + warn on pinned skips
Comment thread
fAnselmi-Ledger marked this conversation as resolved.
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,24 @@ describe("RequirementAccumulator", () => {
expect(result.trustedNames).toEqual(["name"]);
});

it("trustedNameAltRefs is deduplicated but never stripped by the ALT priority dedup", () => {
const accumulator = new RequirementAccumulator();
accumulator.addTrustedNameAltRef("ALT", 5);
accumulator.addTrustedNameAltRef("ALT", 5);
// The same entry also requested through a higher-priority ALT bucket:
// trustedNameAltRefs is a marker set, so it must survive build()'s
// cross-bucket strip untouched.
accumulator.addTokenAccountStateAltRef("ALT", 5);

const result = accumulator.build();
expect(result.trustedNameAltRefs).toEqual([
{ altAddress: "ALT", entryIndex: 5 },
]);
expect(result.tokenAccountStateAltRefs).toEqual([
{ altAddress: "ALT", entryIndex: 5 },
]);
});

it("preserves first-seen insertion order", () => {
const accumulator = new RequirementAccumulator();
accumulator.addTokenInfo("c");
Expand Down Expand Up @@ -137,6 +155,7 @@ describe("RequirementAccumulator", () => {
tokenAccountStates: [],
altResolutions: [],
trustedNames: [],
trustedNameAltRefs: [],
tokenAmountRefs: [],
tokenAmountAltRefs: [],
tokenAccountStateAltRefs: [],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ export class RequirementAccumulator {
private readonly tokenAmountAltRefs = new OrderedSet<AltEntryKey>();
private readonly mintAltRefs = new OrderedSet<AltEntryKey>();
private readonly tokenAccountStateAltRefs = new OrderedSet<AltEntryKey>();
private readonly trustedNameAltRefs = new OrderedSet<AltEntryKey>();

addInstructionInfo(programId: string, discriminator: string): void {
this.instructionInfos.add(`${programId}:${discriminator}`, {
Expand Down Expand Up @@ -59,6 +60,21 @@ export class RequirementAccumulator {
this.trustedNames.add(address, address);
}

/**
* Marks `(altAddress, entryIndex)` as a trusted-name target for an
* ALT-supplied slot. This is not a fifth ALT_RESOLUTION requester: the slot
* is already covered by `altResolutionRule`'s DISPLAY_FIELD pass (or by a
* higher-priority ALT bucket), so this set is consulted, not stripped, in
* the provide phase β€” once any of the other loops resolves the entry, its
* address gets a TRUSTED_NAME fetch too.
*/
addTrustedNameAltRef(altAddress: string, entryIndex: number): void {
this.trustedNameAltRefs.add(`${altAddress}:${entryIndex}`, {
altAddress,
entryIndex,
});
}

addTokenAmountRef(address: string): void {
this.tokenAmountRefs.add(address, address);
}
Expand Down Expand Up @@ -126,6 +142,7 @@ export class RequirementAccumulator {
!tokenAmountKeys.has(altKey(k)),
),
trustedNames: this.trustedNames.values(),
trustedNameAltRefs: this.trustedNameAltRefs.values(),
tokenAmountRefs: this.tokenAmountRefs
.values()
.filter((address) => !tokenAccountKeys.has(address)),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ export function accountReset(opts: {
return {
account_index: opts.accountIndex,
require_pre_balance_zero: opts.requirePreBalanceZero,
value_kind: opts.valueKind ?? "native",
value_kind: opts.valueKind ?? "NATIVE",
token: opts.token,
require_native_pre_balance_zero: opts.requireNativePreBalanceZero,
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -236,12 +236,12 @@ describe("buildRequirements", () => {
accountResets: [
accountReset({
accountIndex: 0,
valueKind: "native",
valueKind: "NATIVE",
requirePreBalanceZero: true,
}),
accountReset({
accountIndex: 0,
valueKind: "splToken",
valueKind: "SPL_TOKEN",
requirePreBalanceZero: true,
token: { kind: "DIRECT" },
}),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -183,10 +183,10 @@ describe("fromCalValueFlowPort optional account strategy", () => {
});

describe("fromCalAccountReset", () => {
it('maps value_kind "native" with no token', () => {
it('maps value_kind "NATIVE" with no token', () => {
const out = fromCalAccountReset({
account_index: 2,
value_kind: "native",
value_kind: "NATIVE",
});
expect(out.accountIndex).toBe(2);
expect(out.valueKind).toBe(ValueKind.NATIVE);
Expand All @@ -195,10 +195,10 @@ describe("fromCalAccountReset", () => {
expect(out.requirePreBalanceZero).toBe(false);
});

it('maps value_kind "splToken" with a token reference', () => {
it('maps value_kind "SPL_TOKEN" with a token reference', () => {
const out = fromCalAccountReset({
account_index: 1,
value_kind: "splToken",
value_kind: "SPL_TOKEN",
token: { kind: "RESOLVE", account_index: 3 },
require_pre_balance_zero: true,
});
Expand All @@ -211,7 +211,7 @@ describe("fromCalAccountReset", () => {
it("maps requireNativePreBalanceZero", () => {
const out = fromCalAccountReset({
account_index: 0,
value_kind: "native",
value_kind: "NATIVE",
require_native_pre_balance_zero: true,
});
expect(out.requireNativePreBalanceZero).toBe(true);
Expand All @@ -229,9 +229,9 @@ describe("fromCalAccountReset", () => {
).toThrow(/unknown or missing ACCOUNT_RESET value_kind/);
});

it("rejects splToken without a token field as a decode error", () => {
it("rejects SPL_TOKEN without a token field as a decode error", () => {
expect(() =>
fromCalAccountReset({ account_index: 0, value_kind: "splToken" }),
fromCalAccountReset({ account_index: 0, value_kind: "SPL_TOKEN" }),
).toThrow(/missing the required TOKEN field/);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -212,8 +212,8 @@ export function fromCalOwnerAssociation(
}

const VALUE_KIND_BY_NAME: Readonly<Record<string, ValueKind>> = {
splToken: ValueKind.SPL_TOKEN,
native: ValueKind.NATIVE,
SPL_TOKEN: ValueKind.SPL_TOKEN,
NATIVE: ValueKind.NATIVE,
};

export function fromCalAccountReset(
Expand All @@ -233,7 +233,7 @@ export function fromCalAccountReset(
}
if (valueKind === ValueKind.SPL_TOKEN && !reset.token) {
decodeError(
"ACCOUNT_RESET with value_kind 'splToken' is missing the required TOKEN field",
"ACCOUNT_RESET with value_kind 'SPL_TOKEN' is missing the required TOKEN field",
);
}
return {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,15 @@ export type DescriptorRequirements = {
tokenAccountStates: string[];
altResolutions: AltEntryKey[];
trustedNames: string[];
/**
* ALT-supplied slots targeted by a `PARAM_TRUSTED_NAME` / `PARAM_ACCOUNT`
* display field. A marker set, not an ALT_RESOLUTION requester: the slot's
* resolution is already requested by whichever of `altResolutions` /
* `tokenAmountAltRefs` / `tokenAccountStateAltRefs` / `mintAltRefs` covers
* it. Once the provide phase resolves the entry, it fetches a TRUSTED_NAME
* for the resulting address too.
*/
trustedNameAltRefs: AltEntryKey[];
/**
* PARAM_TOKEN_AMOUNT.TOKEN refs (ACCOUNT_PATH, non-ALT, not in mintBindings).
* Try TOKEN_INFO first at fetch time; fall back to TOKEN_ACCOUNT_STATE if it fails.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,10 +47,12 @@ import { resolvePortAccountIndex } from "@internal/app-binder/clear-sign/require
*
* Deliberately excluded, to keep device heap use down: read-only ALT accounts
* that no port, token reference, display field, association or reset names. The
* only site that reads them is `collect_all_accounts`, and a slot missing there
* only weakens `condition_account_used_elsewhere` β€” it cannot cost merge
* compaction, it can only make the device show more screens, never fewer and
* never a wrong value.
* only site that reads them is `collect_all_accounts`, and `ACCOUNT_USED_ELSEWHERE`
* is the only predicate that consults it β€” and that predicate is not
* unresolvable (it is never three-valued, unlike a port left unresolved by a
* missing descriptor, see spec/device/tlv_structs.md#unevaluable-predicates,
* G-051), so a slot missing there can only make the device show more screens,
* never fewer and never a wrong value. It cannot cost merge compaction.
*/
export function applyAltResolutionRule(
parsed: ParsedInstruction,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -140,4 +140,62 @@ describe("applyTrustedNameRule", () => {
);
expect(unresolved).toEqual([]);
});

it("falls back to trustedNameAltRefs for an ALT-supplied slot", () => {
const parsed: ParsedInstruction = {
info: {
typePool: [],
rootType: 0,
mintAssociations: [],
ownerAssociations: [],
},
valueFlowPorts: [],
accountResets: [],
displayFields: [
{
paramType: PARAM_TYPE_TRUSTED_NAME,
value: {
source: ValueSource.ACCOUNT_PATH,
payload: Uint8Array.of(0),
},
},
],
hideRules: [],
};
const instruction: RequirementInstruction = {
programId: "P",
accounts: [
{
address: undefined,
altRef: { altAddress: "ALT", entryIndex: 2 },
isWritable: false,
isSigner: false,
},
],
data: new Uint8Array(),
};
const accumulator = new RequirementAccumulator();
applyTrustedNameRule(parsed, instruction, accumulator);
const result = accumulator.build();
expect(result.trustedNames).toEqual([]);
expect(result.trustedNameAltRefs).toEqual([
{ altAddress: "ALT", entryIndex: 2 },
]);
});

it("does not record an ALT ref for a CONSTANT or ARGUMENT_PATH value", () => {
const result = run(
[
{
paramType: PARAM_TYPE_TRUSTED_NAME,
value: {
source: ValueSource.ARGUMENT_PATH,
payload: new Uint8Array(),
},
},
],
[],
);
expect(result).toEqual([]);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,10 @@ import {
type ParsedInstruction,
} from "@internal/app-binder/clear-sign/requirements/records";
import { type RequirementAccumulator } from "@internal/app-binder/clear-sign/requirements/RequirementAccumulator";
import { resolvePubkeyValue } from "@internal/app-binder/clear-sign/requirements/valueResolution";
import {
altRefForPubkeyValue,
resolvePubkeyValue,
} from "@internal/app-binder/clear-sign/requirements/valueResolution";
import {
type Bs58Encoder,
DefaultBs58Encoder,
Expand All @@ -16,6 +19,12 @@ import {
* that may have a CAL name. For `PARAM_ACCOUNT` this is best-effort: the device
* shows the name if a descriptor is found and falls back to the base58 address
* otherwise.
*
* A field targeting an ALT-supplied slot has no address yet at build time β€”
* `resolvePubkeyValue` misses β€” so it is recorded as a `trustedNameAltRef`
* instead: `altResolutionRule` already requests this slot's `ALT_RESOLUTION`,
* and the provide phase fetches the TRUSTED_NAME once that resolution comes
* back.
*/
export function applyTrustedNameRule(
parsed: ParsedInstruction,
Expand All @@ -32,6 +41,13 @@ export function applyTrustedNameRule(
continue;
}
const target = resolvePubkeyValue(field.value, instruction, bs58Encoder);
if (target !== undefined) accumulator.addTrustedName(target);
if (target !== undefined) {
accumulator.addTrustedName(target);
continue;
}
const altRef = altRefForPubkeyValue(field.value, instruction);
if (altRef !== undefined) {
accumulator.addTrustedNameAltRef(altRef.altAddress, altRef.entryIndex);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,11 @@ export type ProvideInstructionSubstructureCommandArgs = {
* HIDE_RULE / ACCOUNT_RESET) referenced by the current `INSTRUCTION_INFO`.
*
* The caller pre-builds the wire payload β€” a 1-byte substructure-type selector
* followed by the substructure TLV (no length prefix; the device recovers the
* total length from the chunk flags) β€” and splits it into ≀255-byte chunks.
* followed by the substructure TLV β€” and splits it into ≀255-byte chunks. The
* `SUBSTRUCT_TYPE β€– uint32be length β€– TLV` framing is absent on the wire (the
* device recovers the total length from the chunk flags) but the device still
* folds that framing into the running `SUBSTRUCTURES_HASH`, so each call must
* carry exactly one substructure β€” never two packed together.
*/
export class ProvideInstructionSubstructureCommand
implements
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,7 @@ export class ProvisionGenericClearSignDeviceAction extends XStateDeviceAction<
tokenAmountAltRefs: [],
tokenAccountStateAltRefs: [],
mintAltRefs: [],
trustedNameAltRefs: [],
},
}),
onDone: [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@ export type ChallengeBoundRequirements = Pick<
tokenAccountStateAltRefs: AltEntryKey[];
/** ALT-backed MINT entries from MINT_ASSOCIATIONS; require TOKEN_INFO via hold-and-conditionally-stream. */
mintAltRefs: AltEntryKey[];
/** ALT-supplied slots targeted by a trusted-name display field; see {@link DescriptorRequirements.trustedNameAltRefs}. */
trustedNameAltRefs: AltEntryKey[];
};

/**
Expand Down Expand Up @@ -110,6 +112,7 @@ export class BuildGenericClearSignContextTask {
tokenAmountAltRefs: [],
tokenAccountStateAltRefs: [],
mintAltRefs: [],
trustedNameAltRefs: [],
},
unrecognizedProgramIds: [],
staleDescriptor: false,
Expand Down Expand Up @@ -316,6 +319,7 @@ export class BuildGenericClearSignContextTask {
tokenAmountAltRefs: requirements.tokenAmountAltRefs,
tokenAccountStateAltRefs: requirements.tokenAccountStateAltRefs,
mintAltRefs: requirements.mintAltRefs,
trustedNameAltRefs: requirements.trustedNameAltRefs,
};

this.logger.debug("[run] built clear-sign context", {
Expand Down
Loading
Loading