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

Use omitDefault to fix Amino JSON converter for Instantiate2 #1509

Merged
merged 3 commits into from
Nov 15, 2023
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 CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,15 +12,20 @@ and this project adheres to
`sign`/`signAndBroadcast`/`signAndBroadcastSync` and related functions now
have an additional parameter to specify the timeout height. After this height,
a signed transaction will be considered invalid by the chain. ([#1489])
- @cosmjs/amino: Export `omitDefault` to help build Amino JSON converters.

[#1489]: https://github.com/cosmos/cosmjs/pull/1489

### Fixed

- @cosmjs/stargate: Handle key value pairs in tx search correctly if the value
is a numeric value. ([#1462])
- @cosmjs/cosmwasm-stargate: Make `fix_msg` optional in
`AminoMsgInstantiateContract2` and omit default in the Amino JSON converter to
fix Amino JSON signing for MsgInstantiateContract2. ([#1509])

[#1462]: https://github.com/cosmos/cosmjs/issues/1462
[#1509]: https://github.com/cosmos/cosmjs/pull/1509

### Changed

Expand Down
1 change: 1 addition & 0 deletions packages/amino/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ export {
encodeSecp256k1Pubkey,
} from "./encoding";
export { createMultisigThresholdPubkey } from "./multisig";
export { omitDefault } from "./omitdefault";
export { makeCosmoshubPath } from "./paths";
export {
Ed25519Pubkey,
Expand Down
23 changes: 23 additions & 0 deletions packages/amino/src/omitdefault.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import { omitDefault } from "./omitdefault";

describe("omitDefault", () => {
it("works for numbers", () => {
expect(omitDefault(17)).toEqual(17);
expect(omitDefault(0)).toEqual(undefined);
});

it("works for bigint", () => {
expect(omitDefault(17n)).toEqual(17n);
expect(omitDefault(0n)).toEqual(undefined);
});

it("works for boolean", () => {
expect(omitDefault(true)).toEqual(true);
expect(omitDefault(false)).toEqual(undefined);
});

it("works for strings", () => {
expect(omitDefault("hi")).toEqual("hi");
expect(omitDefault("")).toEqual(undefined);
});
});
18 changes: 18 additions & 0 deletions packages/amino/src/omitdefault.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
/**
* Returns the given input. If the input is the default value
* of protobuf, undefined is retunred. Use this when creating Amino JSON converters.
*/
export function omitDefault<T extends string | number | bigint | boolean>(input: T): T | undefined {
switch (typeof input) {
case "string":
return input === "" ? undefined : input;
case "number":
return input === 0 ? undefined : input;
case "bigint":
return input === BigInt(0) ? undefined : input;
case "boolean":
return !input ? undefined : input;
default:
throw new Error(`Got unsupported type '${typeof input}'`);
}
}
36 changes: 34 additions & 2 deletions packages/cosmwasm-stargate/src/modules/wasm/aminomessages.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ describe("AminoTypes", () => {
funds: coins(1234, "ucosm"),
admin: "cosmos10dyr9899g6t0pelew4nvf4j5c3jcgv0r73qga5",
salt: toBase64(toUtf8("salt")),
fix_msg: false,
fix_msg: undefined,
},
};
expect(aminoMsg).toEqual(expected);
Expand Down Expand Up @@ -191,7 +191,39 @@ describe("AminoTypes", () => {
funds: coins(1234, "ucosm"),
admin: undefined,
salt: toBase64(toUtf8("salt")),
fix_msg: false,
fix_msg: undefined,
},
};
expect(aminoMsg).toEqual(expected);
}

// With fixMsg=true (typically not needed)
{
const msg: MsgInstantiateContract2 = {
sender: "cosmos1pkptre7fdkl6gfrzlesjjvhxhlc3r4gmmk8rs6",
codeId: BigInt("12345"),
label: "sticky",
msg: toUtf8(`{"foo":"bar"}`),
funds: coins(1234, "ucosm"),
admin: "cosmos10dyr9899g6t0pelew4nvf4j5c3jcgv0r73qga5",
salt: toUtf8("salt"),
fixMsg: true,
};
const aminoMsg = new AminoTypes(createWasmAminoConverters()).toAmino({
typeUrl: "/cosmwasm.wasm.v1.MsgInstantiateContract2",
value: msg,
});
const expected: AminoMsgInstantiateContract2 = {
type: "wasm/MsgInstantiateContract2",
value: {
sender: "cosmos1pkptre7fdkl6gfrzlesjjvhxhlc3r4gmmk8rs6",
code_id: "12345",
label: "sticky",
msg: { foo: "bar" },
funds: coins(1234, "ucosm"),
admin: "cosmos10dyr9899g6t0pelew4nvf4j5c3jcgv0r73qga5",
salt: toBase64(toUtf8("salt")),
fix_msg: true,
},
};
expect(aminoMsg).toEqual(expected);
Expand Down
16 changes: 10 additions & 6 deletions packages/cosmwasm-stargate/src/modules/wasm/aminomessages.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
/* eslint-disable @typescript-eslint/naming-convention */
import { omitDefault } from "@cosmjs/amino";
import { fromBase64, fromUtf8, toBase64, toUtf8 } from "@cosmjs/encoding";
import { AminoConverters, Coin } from "@cosmjs/stargate";
import {
Expand Down Expand Up @@ -141,8 +142,11 @@ export interface AminoMsgInstantiateContract2 {
readonly admin?: string;
/** Arbitrary Base64-encoded value provided by the sender */
readonly salt: string;
/** Whether or not to include the msg value into the hash for the address */
readonly fix_msg: boolean;
/**
* Whether or not to include the msg value into the hash for the address.
* Unset means false. This should always be unset/false (https://medium.com/cosmwasm/dev-note-3-limitations-of-instantiate2-and-how-to-deal-with-them-a3f946874230).
*/
readonly fix_msg?: boolean;
};
}

Expand Down Expand Up @@ -248,7 +252,7 @@ export function createWasmAminoConverters(): AminoConverters {
label: label,
msg: JSON.parse(fromUtf8(msg)),
funds: funds,
admin: admin || undefined,
admin: omitDefault(admin),
}),
fromAmino: ({
sender,
Expand Down Expand Up @@ -283,9 +287,9 @@ export function createWasmAminoConverters(): AminoConverters {
label: label,
msg: JSON.parse(fromUtf8(msg)),
funds: funds,
admin: admin || undefined,
admin: omitDefault(admin),
salt: toBase64(salt),
fix_msg: fixMsg,
fix_msg: omitDefault(fixMsg),
}),
fromAmino: ({
sender,
Expand All @@ -304,7 +308,7 @@ export function createWasmAminoConverters(): AminoConverters {
funds: [...funds],
admin: admin ?? "",
salt: fromBase64(salt),
fixMsg: fix_msg,
fixMsg: fix_msg ?? false,
}),
},
"/cosmwasm.wasm.v1.MsgUpdateAdmin": {
Expand Down
41 changes: 40 additions & 1 deletion packages/cosmwasm-stargate/src/signingcosmwasmclient.spec.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/* eslint-disable @typescript-eslint/naming-convention */
import { Secp256k1HdWallet } from "@cosmjs/amino";
import { sha256 } from "@cosmjs/crypto";
import { Random, sha256 } from "@cosmjs/crypto";
import { toHex, toUtf8 } from "@cosmjs/encoding";
import { decodeTxRaw, DirectSecp256k1HdWallet, Registry } from "@cosmjs/proto-signing";
import {
Expand Down Expand Up @@ -343,6 +343,45 @@ describe("SigningCosmWasmClient", () => {
expect(contractAddress).toEqual(expectedAddress);
client.disconnect();
});

it("works with Amino JSON signing", async () => {
pendingWithoutWasmd();
const aminoJsonWallet = await Secp256k1HdWallet.fromMnemonic(alice.mnemonic, {
prefix: wasmd.prefix,
});
const client = await SigningCosmWasmClient.connectWithSigner(
wasmd.endpoint,
aminoJsonWallet,
defaultSigningClientOptions,
);
const { codeId } = await client.upload(alice.address0, getHackatom().data, defaultUploadFee);
const funds = [coin(1234, "ucosm"), coin(321, "ustake")];
const salt = Random.getBytes(64);
const msg = {
verifier: alice.address0,
beneficiary: makeRandomAddress(),
};

const { contractAddress } = await client.instantiate2(
alice.address0,
codeId,
salt,
msg,
"My cool label--",
defaultInstantiateFee,
{
memo: "Let's see if the memo is used",
funds: funds,
},
);

const ucosmBalance = await client.getBalance(contractAddress, "ucosm");
const ustakeBalance = await client.getBalance(contractAddress, "ustake");
expect(ucosmBalance).toEqual(funds[0]);
expect(ustakeBalance).toEqual(funds[1]);

client.disconnect();
});
});

describe("updateAdmin", () => {
Expand Down
18 changes: 1 addition & 17 deletions packages/stargate/src/modules/ibc/aminomessages.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/* eslint-disable @typescript-eslint/naming-convention */
import { AminoMsg, Coin } from "@cosmjs/amino";
import { AminoMsg, Coin, omitDefault } from "@cosmjs/amino";
import { MsgTransfer } from "cosmjs-types/ibc/applications/transfer/v1/tx";

import { AminoConverters } from "../../aminotypes";
Expand Down Expand Up @@ -45,22 +45,6 @@ export function isAminoMsgTransfer(msg: AminoMsg): msg is AminoMsgTransfer {
return msg.type === "cosmos-sdk/MsgTransfer";
}

function omitDefault<T extends string | number | bigint>(input: T): T | undefined {
if (typeof input === "string") {
return input === "" ? undefined : input;
}

if (typeof input === "number") {
return input === 0 ? undefined : input;
}

if (typeof input === "bigint") {
return input === BigInt(0) ? undefined : input;
}

throw new Error(`Got unsupported type '${typeof input}'`);
}

export function createIbcAminoConverters(): AminoConverters {
return {
"/ibc.applications.transfer.v1.MsgTransfer": {
Expand Down