Skip to content

Commit 64607e2

Browse files
fix(soroban): surface the CAP-85 note earlier and name invalid pairings
Follow-up polish on the auth-entry review UI plus clearer diagnostics for malformed contract creations. The externally-managed-executable note now leads the auth entry's info block instead of trailing it, so the caveat is read before the details it qualifies. Its copy drops the positional reference ("the owner contract above") and names the row instead ("the executable owner"), which stays correct in both placements -- the Operations view still renders the note after the executable rows, where the existing Operations__warning spacing pattern applies.
1 parent dad76b0 commit 64607e2

7 files changed

Lines changed: 89 additions & 12 deletions

File tree

extension/src/popup/components/AuthEntry/index.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -164,6 +164,7 @@ export const AuthEntries = ({ entries }: AuthEntriesProps) => {
164164
<span>{t("Contract creation")}</span>
165165
</div>
166166
<div className="AuthEntry__InfoBlock">
167+
<ExternalExecutableNote />
167168
{detail.address && (
168169
<KeyValueList
169170
operationKey={t("Contract Address")}
@@ -194,7 +195,6 @@ export const AuthEntries = ({ entries }: AuthEntriesProps) => {
194195
operationValue={truncateString(detail.salt)}
195196
/>
196197
)}
197-
<ExternalExecutableNote />
198198
{detail.args && <KeyValueInvokeHostFnArgs args={detail.args} />}
199199
</div>
200200
</React.Fragment>

extension/src/popup/components/AuthEntry/styles.scss

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,33 @@
4141
}
4242

4343
&__InfoBlock {
44+
// The executable note leads the block here (rather than trailing the rows
45+
// as it does in the Operations view), so it carries its spacing below.
46+
> .ExecutableNote {
47+
margin-top: 0;
48+
margin-bottom: pxToRem(12px);
49+
}
50+
51+
// Only the top-level Parameters block needs separating; the invoke case
52+
// nests it inside _Inner, which already provides its own spacing.
53+
> .Operations__pair--invoke {
54+
margin-top: pxToRem(12px);
55+
}
56+
57+
.Operations--header {
58+
display: flex;
59+
align-items: center;
60+
color: var(--sds-clr-gray-11);
61+
margin-bottom: pxToRem(12px);
62+
63+
svg {
64+
flex-shrink: 0;
65+
width: pxToRem(16px);
66+
height: pxToRem(16px);
67+
margin-right: pxToRem(8px);
68+
}
69+
}
70+
4471
&_Inner {
4572
padding: pxToRem(12px) pxToRem(16px);
4673
border-radius: pxToRem(16px);
@@ -187,6 +214,7 @@
187214

188215
.AuthEntryContent {
189216
width: 100%;
217+
padding-top: pxToRem(12px);
190218
animation: fadeIn 0.2s ease-in-out;
191219
}
192220

extension/src/popup/components/signTransaction/Operations/KeyVal/index.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -470,7 +470,7 @@ export const ExternalExecutableNote = () => {
470470
<Icon.InfoCircle aria-hidden="true" />
471471
<span>
472472
{t(
473-
"This contract's code is managed by the owner contract above and can change after you sign.",
473+
"This contract's code is managed by the executable owner and can change after you sign.",
474474
)}
475475
</span>
476476
</div>

extension/src/popup/helpers/__tests__/soroban.test.js

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,47 @@ describe("getInvocationArgs", () => {
180180
salt: "0".repeat(64),
181181
});
182182
});
183+
it("explains which executable/preimage pairing was invalid when it throws", () => {
184+
const assetPreimage = xdr.ContractIdPreimage.contractIdPreimageFromAsset(
185+
xdr.Asset.assetTypeNative(),
186+
);
187+
const addressPreimage =
188+
xdr.ContractIdPreimage.contractIdPreimageFromAddress(
189+
new xdr.ContractIdPreimageFromAddress({
190+
address: new Address(TEST_PUBLIC_KEY).toScAddress(),
191+
salt: Buffer.alloc(32),
192+
}),
193+
);
194+
195+
const build = (executable, contractIdPreimage) =>
196+
new xdr.SorobanAuthorizedInvocation({
197+
function:
198+
xdr.SorobanAuthorizedFunction.sorobanAuthorizedFunctionTypeCreateContractHostFn(
199+
new xdr.CreateContractArgs({ contractIdPreimage, executable }),
200+
),
201+
subInvocations: [],
202+
});
203+
204+
// wasm code must be deployed from an address, never derived from an asset
205+
expect(() =>
206+
getInvocationArgs(
207+
build(
208+
xdr.ContractExecutable.contractExecutableWasm(Buffer.alloc(32)),
209+
assetPreimage,
210+
),
211+
),
212+
).toThrow(/wasm executable.*contractIdPreimageFromAsset/);
213+
214+
// and a SAC is only ever derived from an asset
215+
expect(() =>
216+
getInvocationArgs(
217+
build(
218+
xdr.ContractExecutable.contractExecutableStellarAsset(),
219+
addressPreimage,
220+
),
221+
),
222+
).toThrow(/Stellar asset executable.*contractIdPreimageFromAddress/);
223+
});
183224
it("marks an invocation it cannot parse as unrecognized instead of throwing", () => {
184225
// A wasm executable paired with an asset preimage is decodable XDR but a
185226
// nonsensical combination -- the kind of thing a future protocol arm or a

extension/src/popup/helpers/soroban.ts

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -865,10 +865,15 @@ export function getInvocationArgs(
865865

866866
switch (exec.type) {
867867
case "contractExecutableWasm": {
868-
const details = xdr.expectUnionVariant(
869-
preimage,
870-
"contractIdPreimageFromAddress",
871-
).fromAddress;
868+
// A wasm executable must be paired with an address preimage: the
869+
// contract id is derived from deployer + salt. The two arms are
870+
// independent in XDR, so the invalid pairings are representable.
871+
if (preimage.type !== "contractIdPreimageFromAddress") {
872+
throw new Error(
873+
`creation function appears invalid: a wasm executable is paired with ${preimage.type} (should be wasm+address or token+asset)`,
874+
);
875+
}
876+
const details = preimage.fromAddress;
872877

873878
const contractDetails = {
874879
type: "wasm",
@@ -887,12 +892,15 @@ export function getInvocationArgs(
887892
}
888893

889894
case "contractExecutableStellarAsset": {
895+
// A SAC is only ever derived from the asset it wraps.
896+
if (preimage.type !== "contractIdPreimageFromAsset") {
897+
throw new Error(
898+
`creation function appears invalid: a Stellar asset executable is paired with ${preimage.type} (should be wasm+address or token+asset)`,
899+
);
900+
}
890901
const sacDetails = {
891902
type: "sac",
892-
asset: Asset.fromOperation(
893-
xdr.expectUnionVariant(preimage, "contractIdPreimageFromAsset")
894-
.fromAsset,
895-
).toString(),
903+
asset: Asset.fromOperation(preimage.fromAsset).toString(),
896904
} as FnArgsCreateSac;
897905

898906
if (isCreateV2) {

extension/src/popup/locales/en/translation.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -636,7 +636,7 @@
636636
"This authorization is for {{address}}.": "This authorization is for {{address}}.",
637637
"This can be used to sign arbitrary transaction hashes without having to decode them first.": "This can be used to sign arbitrary transaction hashes without having to decode them first.",
638638
"This collectible is hidden": "This collectible is hidden",
639-
"This contract's code is managed by the owner contract above and can change after you sign.": "This contract's code is managed by the owner contract above and can change after you sign.",
639+
"This contract's code is managed by the executable owner and can change after you sign.": "This contract's code is managed by the executable owner and can change after you sign.",
640640
"This is not a valid contract id.": "This is not a valid contract id.",
641641
"This message is too large for your Ledger to display. Ask the site for a shorter message.": "This message is too large for your Ledger to display. Ask the site for a shorter message.",
642642
"This site does not appear safe for the following reasons": "This site does not appear safe for the following reasons",

extension/src/popup/locales/pt/translation.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -636,7 +636,7 @@
636636
"This authorization is for {{address}}.": "Esta autorização é para {{address}}.",
637637
"This can be used to sign arbitrary transaction hashes without having to decode them first.": "Isso pode ser usado para assinar hashes de transação arbitrários sem precisar decodificá-los primeiro.",
638638
"This collectible is hidden": "Este colecionável está oculto",
639-
"This contract's code is managed by the owner contract above and can change after you sign.": "O código deste contrato é gerenciado pelo contrato proprietário acima e pode mudar depois que você assinar.",
639+
"This contract's code is managed by the executable owner and can change after you sign.": "O código deste contrato é gerenciado pelo proprietário do executável e pode mudar depois que você assinar.",
640640
"This is not a valid contract id.": "Este não é um ID de contrato válido.",
641641
"This message is too large for your Ledger to display. Ask the site for a shorter message.": "Esta mensagem é grande demais para o seu Ledger exibir. Peça ao site uma mensagem mais curta.",
642642
"This site does not appear safe for the following reasons": "Este site não parece seguro pelos seguintes motivos",

0 commit comments

Comments
 (0)