Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG] fixes an incorrect reference to the old create contract class for v2 #1661

Merged
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
61 changes: 61 additions & 0 deletions extension/src/popup/helpers/__tests__/soroban.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
import { Address, Keypair, xdr } from "stellar-sdk";

import { getInvocationArgs } from "../soroban";
import { TEST_PUBLIC_KEY } from "popup/__testHelpers__";

describe("getInvocationArgs", () => {
it("can render a create contract v1", () => {
const assetCode = "KHL";
const assetType = new xdr.AlphaNum4({
assetCode: Buffer.from(assetCode),
issuer: Keypair.fromPublicKey(TEST_PUBLIC_KEY).xdrAccountId(),
});
const args = new xdr.CreateContractArgs({
contractIdPreimage: xdr.ContractIdPreimage.contractIdPreimageFromAsset(
xdr.Asset.assetTypeCreditAlphanum4(assetType),
),
executable: xdr.ContractExecutable.contractExecutableStellarAsset(),
});
const authorizedFn =
xdr.SorobanAuthorizedFunction.sorobanAuthorizedFunctionTypeCreateContractHostFn(
args,
);
const authorizedInvocation = new xdr.SorobanAuthorizedInvocation({
function: authorizedFn,
subInvocations: [],
});
const invocationArgs = getInvocationArgs(authorizedInvocation);
expect(invocationArgs).toEqual({
type: "sac",
asset: `${assetCode}:${TEST_PUBLIC_KEY}`,
});
});
it("can render a create contract v2", () => {
const assetCode = "KHL";
const assetType = new xdr.AlphaNum4({
assetCode: Buffer.from(assetCode),
issuer: Keypair.fromPublicKey(TEST_PUBLIC_KEY).xdrAccountId(),
});
const args = new xdr.CreateContractArgsV2({
contractIdPreimage: xdr.ContractIdPreimage.contractIdPreimageFromAsset(
xdr.Asset.assetTypeCreditAlphanum4(assetType),
),
executable: xdr.ContractExecutable.contractExecutableStellarAsset(),
constructorArgs: [new Address(TEST_PUBLIC_KEY).toScVal()],
});
const authorizedFn =
xdr.SorobanAuthorizedFunction.sorobanAuthorizedFunctionTypeCreateContractV2HostFn(
args,
);
const authorizedInvocation = new xdr.SorobanAuthorizedInvocation({
function: authorizedFn,
subInvocations: [],
});
const invocationArgs = getInvocationArgs(authorizedInvocation);
expect(invocationArgs).toEqual({
type: "sac",
asset: `${assetCode}:${TEST_PUBLIC_KEY}`,
args: args.constructorArgs(),
});
});
});
36 changes: 30 additions & 6 deletions extension/src/popup/helpers/soroban.ts
Original file line number Diff line number Diff line change
Expand Up @@ -252,8 +252,9 @@ export function buildInvocationTree(root: xdr.SorobanAuthorizedInvocation) {
}

// sorobanAuthorizedFunctionTypeCreateContractHostFn
case 2:
case 1: {
const _inner = inner as xdr.CreateContractArgs;
const _inner = inner as xdr.CreateContractArgs | xdr.CreateContractArgsV2;
output.type = "create";
output.args = {} as {
type: string;
Expand Down Expand Up @@ -291,6 +292,11 @@ export function buildInvocationTree(root: xdr.SorobanAuthorizedInvocation) {
hash: exec.wasmHash().toString("hex"),
address: Address.fromScAddress(details.address()).toString(),
};
// create contract V2
if (fn.switch().value === 2) {
const v2Args = _inner as xdr.CreateContractArgsV2;
output.args.constructorArgs = v2Args.constructorArgs();
}
break;
}

Expand All @@ -300,6 +306,11 @@ export function buildInvocationTree(root: xdr.SorobanAuthorizedInvocation) {
output.args.asset = Asset.fromOperation(
preimage.fromAsset(),
).toString();
// create contract V2
if (fn.switch().value === 2) {
const v2Args = _inner as xdr.CreateContractArgsV2;
output.args.constructorArgs = v2Args.constructorArgs();
}
break;

default:
Expand Down Expand Up @@ -436,6 +447,7 @@ export interface FnArgsCreateWasm {
export interface FnArgsCreateSac {
type: "sac";
asset: string;
args?: xdr.ScVal[];
}

type InvocationArgs = FnArgsInvoke | FnArgsCreateWasm | FnArgsCreateSac;
Expand All @@ -444,7 +456,7 @@ const isInvocationArg = (
invocation: InvocationArgs | undefined,
): invocation is InvocationArgs => !!invocation;

function getInvocationArgs(
export function getInvocationArgs(
invocation: xdr.SorobanAuthorizedInvocation,
): InvocationArgs | undefined {
const fn = invocation.function();
Expand All @@ -465,7 +477,10 @@ function getInvocationArgs(
// sorobanAuthorizedFunctionTypeCreateContractHostFn
case 2:
case 1: {
const _invocation = fn.createContractHostFn();
const _invocation =
fn.switch().value === 2
? fn.createContractV2HostFn()
: fn.createContractHostFn();
const [exec, preimage] = [
_invocation.executable(),
_invocation.contractIdPreimage(),
Expand Down Expand Up @@ -493,11 +508,20 @@ function getInvocationArgs(
}

// contractExecutableStellarAsset
case 1:
return {
case 1: {
const sacDetails = {
type: "sac",
asset: Asset.fromOperation(preimage.fromAsset()).toString(),
};
} as FnArgsCreateSac;

if (fn.switch().value === 2) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It wasn't super clear to me through the xdr if this was possible for the SAC case but I think there's no reason to not include this line. If SACs will keep using v1 then this conditional won't ever be hit.

sacDetails.args = (
_invocation as xdr.CreateContractArgsV2
).constructorArgs();
}

return sacDetails;
}

default:
throw new Error(`unknown creation type: ${JSON.stringify(exec)}`);
Expand Down
24 changes: 16 additions & 8 deletions extension/src/popup/views/ReviewAuth/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -351,10 +351,12 @@ const AuthDetail = ({
async function getIsToken() {
try {
const transfers = [];
const isToken = await getIsTokenSpec({
contractId: rootJson.args.source,
networkDetails,
});
const isToken = !rootJson.args.source
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for both the invocation and sub invocation this should account for not having a source in the root args.

? false
: await getIsTokenSpec({
contractId: rootJson.args.source,
networkDetails,
});
if (isToken && rootJson.args.function === "transfer") {
transfers.push({
contractId: rootJson.args.source as string,
Expand All @@ -366,10 +368,12 @@ const AuthDetail = ({
// check for sub transfers
// eslint-disable-next-line no-restricted-syntax
for (const subInvocation of rootJson.invocations) {
const isSubInvokeToken = await getIsTokenSpec({
contractId: subInvocation.args.source,
networkDetails,
});
const isSubInvokeToken = !subInvocation.args.source
? false
: await getIsTokenSpec({
contractId: subInvocation.args.source,
networkDetails,
});
if (isSubInvokeToken && subInvocation.args.function === "transfer") {
transfers.push({
contractId: subInvocation.args.source as string,
Expand All @@ -381,6 +385,7 @@ const AuthDetail = ({
}
setAuthTransfers(transfers);
setCheckingTransfers(false);
setLoading(false);
} catch (error) {
console.error(error);
setCheckingTransfers(false);
Expand All @@ -391,6 +396,7 @@ const AuthDetail = ({
getIsToken();
} else {
setCheckingTransfers(false);
setLoading(false);
}
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [isInvokeContract, rootJsonDepKey]);
Expand Down Expand Up @@ -439,6 +445,7 @@ const AuthDetail = ({
_getTokenDetails();
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [transfersDepKey]);

return (
<div className="AuthDetail" data-testid="AuthDetail">
{isLoading || isCheckingTransfers ? (
Expand Down Expand Up @@ -530,6 +537,7 @@ const AuthDetail = ({
operationKey={t("Asset")}
operationValue={truncateString(detail.asset)}
/>
{detail.args && <KeyValueInvokeHostFnArgs args={detail.args} />}
</div>
</React.Fragment>
))}
Expand Down