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

Remove usage of instanceof when sanitizing wallet-api inputs #404

Merged
merged 7 commits into from
Nov 7, 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: 3 additions & 2 deletions examples/piggybank/src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {
InitName,
ReceiveName,
UpdateContractPayload,
AccountAddress,
} from '@concordium/web-sdk';

export const CONTRACT_NAME = 'PiggyBank';
Expand All @@ -26,7 +27,7 @@ export const deposit = (account: string, index: bigint, subindex = 0n, amount =
detectConcordiumProvider()
.then((provider) => {
provider
.sendTransaction(account, AccountTransactionType.Update, {
.sendTransaction(AccountAddress.fromBase58(account), AccountTransactionType.Update, {
amount: CcdAmount.fromMicroCcd(amount),
address: ContractAddress.create(index, subindex),
receiveName: ReceiveName.fromString(`${CONTRACT_NAME}.insert`),
Expand All @@ -50,7 +51,7 @@ export const smash = (account: string, index: bigint, subindex = 0n) => {
detectConcordiumProvider()
.then((provider) => {
provider
.sendTransaction(account, AccountTransactionType.Update, {
.sendTransaction(AccountAddress.fromBase58(account), AccountTransactionType.Update, {
amount: CcdAmount.fromMicroCcd(0), // This feels weird? Why do I need an amount for a non-payable receive?
address: ContractAddress.create(index, subindex),
receiveName: ReceiveName.fromString(`${CONTRACT_NAME}.smash`),
Expand Down
14 changes: 7 additions & 7 deletions examples/two-step-transfer/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,8 @@
document.getElementById('sendTransfer').addEventListener('click', () =>
provider
.sendTransaction(currentAccountAddress, concordiumSDK.AccountTransactionType.Transfer, {
amount: new concordiumSDK.CcdAmount(1n),
toAddress: new concordiumSDK.AccountAddress(
amount: concordiumSDK.CcdAmount.fromMicroCcd(1n),
toAddress: concordiumSDK.AccountAddress.fromBase58(
'3kBx2h5Y2veb4hZgAJWPrr8RyQESKm5TjzF3ti1QQ4VSYLwK1G'
),
})
Expand Down Expand Up @@ -103,7 +103,7 @@
);
provider
.signMessage(currentAccountAddress, {
data: serializedMessage.toString('hex'),
data: concordiumSDK.Parameter.toHexString(serializedMessage),
schema: SERIALIZATION_HELPER_SCHEMA,
})
.then((sig) => alert(JSON.stringify(sig)))
Expand All @@ -112,7 +112,7 @@
document.getElementById('sendDeposit').addEventListener('click', () =>
provider
.sendTransaction(currentAccountAddress, concordiumSDK.AccountTransactionType.Update, {
amount: new concordiumSDK.CcdAmount(1n),
amount: concordiumSDK.CcdAmount.fromMicroCcd(1n),
contractAddress: {
index: 98n,
subindex: 0n,
Expand All @@ -129,7 +129,7 @@
currentAccountAddress,
concordiumSDK.AccountTransactionType.Update,
{
amount: new concordiumSDK.CcdAmount(1n),
amount: concordiumSDK.CcdAmount.fromMicroCcd(1n),
contractAddress: {
index: 98n,
subindex: 0n,
Expand All @@ -154,8 +154,8 @@
currentAccountAddress,
concordiumSDK.AccountTransactionType.InitContract,
{
amount: new concordiumSDK.CcdAmount(1n),
moduleRef: new concordiumSDK.ModuleReference(
amount: concordiumSDK.CcdAmount.fromMicroCcd(1n),
moduleRef: concordiumSDK.ModuleReference.fromHexString(
'6eab4584bd60b1d82fba3abe16082cde3dbee4c08c9f60b6dc523688bdc421b9'
),
contractName: 'two-step-transfer',
Expand Down
2 changes: 1 addition & 1 deletion examples/two-step-transfer/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
"live-server": "^1.2.2"
},
"scripts": {
"start": "live-server ../two-step-transfer/index.html --mount=/sdk.js:../../node_modules/@concordium/web-sdk/lib/concordium.min.js --mount=/helpers.js:../../packages/browser-wallet-api-helpers/lib/concordiumHelpers.min.js"
"start": "live-server ../two-step-transfer/index.html --mount=/sdk.js:../../node_modules/@concordium/web-sdk/lib/min/concordium.web.min.js --mount=/helpers.js:../../packages/browser-wallet-api-helpers/lib/concordiumHelpers.min.js"
},
"dependencies": {
"@concordium/web-sdk": "^7.0.3"
Expand Down
39 changes: 29 additions & 10 deletions packages/browser-wallet-api/src/compatibility.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import {
SimpleTransferPayload,
SimpleTransferWithMemoPayload,
UpdateCredentialsPayload,
DataBlob,
} from '@concordium/web-sdk';

export type GtuAmount = { microGtuAmount: bigint };
Expand All @@ -42,7 +43,9 @@ export function isGtuAmount(cand: any): cand is GtuAmount {
}

function sanitizeAccountAddress(accountAddress: AccountAddressSource): AccountAddress.Type {
return AccountAddress.instanceOf(accountAddress) ? accountAddress : AccountAddress.fromBase58(accountAddress);
return typeof accountAddress === 'string'
? AccountAddress.fromBase58(accountAddress)
: AccountAddress.fromBase58(accountAddress.address);
}

export type SanitizedSignMessageInput = {
Expand Down Expand Up @@ -92,10 +95,10 @@ export function sanitizeAddCIS2TokensInput(
): SanitizedAddCIS2TokensInput {
const accountAddress = sanitizeAccountAddress(_accountAddress);
let contractAddress: ContractAddress.Type;
if (ContractAddress.instanceOf(contractAddressSource)) {
contractAddress = contractAddressSource;
} else {
if (typeof contractAddressSource === 'bigint') {
contractAddress = ContractAddress.create(contractAddressSource, contractSubindex);
} else {
contractAddress = ContractAddress.create(contractAddressSource.index, contractAddressSource.subindex);
}

return { accountAddress, tokenIds, contractAddress };
Expand Down Expand Up @@ -296,28 +299,44 @@ function sanitizePayload(type: AccountTransactionType, payload: SendTransactionP
source,
} as DeployModulePayload;
}
case AccountTransactionType.Transfer:
case AccountTransactionType.Transfer: {
const p = payload as SimpleTransferPayloadCompat;

const amount = CcdAmount.fromMicroCcd(
isGtuAmount(p.amount) ? p.amount.microGtuAmount : p.amount.microCcdAmount
);
const toAddress = AccountAddress.fromBuffer(p.toAddress.decodedAddress);

return { amount, toAddress } as SimpleTransferPayload;
}
case AccountTransactionType.TransferWithMemo: {
// The "memo" part of transfers have not changes, so these are treated the same.
const p = payload as SimpleTransferPayloadCompat | SimpleTransferWithMemoPayloadCompat;
const p = payload as SimpleTransferWithMemoPayloadCompat;

const amount = CcdAmount.fromMicroCcd(
isGtuAmount(p.amount) ? p.amount.microGtuAmount : p.amount.microCcdAmount
);
const toAddress = AccountAddress.fromBuffer(p.toAddress.decodedAddress);

return { ...p, amount, toAddress } as SimpleTransferPayload | SimpleTransferWithMemoPayload;
const memo = new DataBlob(p.memo.data);

return { amount, toAddress, memo } as SimpleTransferWithMemoPayload;
}
case AccountTransactionType.ConfigureBaker:
case AccountTransactionType.ConfigureDelegation: {
const p = payload as ConfigureBakerPayloadCompat | ConfigureDelegationPayloadCompat;
const stake = p.stake !== undefined ? CcdAmount.fromMicroCcd(p.stake.microCcdAmount) : undefined;
return { ...p, stake } as ConfigureBakerPayload | ConfigureDelegationPayload;
}
case AccountTransactionType.RegisterData:
case AccountTransactionType.RegisterData: {
const p = payload as RegisterDataPayloadCompat;

const data = new DataBlob(p.data.data);

return { data } as RegisterDataPayload;
}
case AccountTransactionType.UpdateCredentials:
// No changes across any API versions.
return payload as RegisterDataPayload | UpdateCredentialsPayload;
return payload as UpdateCredentialsPayload;
default:
// This should never happen, but is here for backwards compatibility.
return payload as SendTransactionPayload;
Expand Down
69 changes: 68 additions & 1 deletion packages/browser-wallet-api/test/compatibility.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ import {
Energy,
EntrypointName,
IdStatementBuilder,
jsonParse,
jsonStringify,
ModuleReference,
OpenStatus,
ReceiveName,
Expand Down Expand Up @@ -391,6 +393,46 @@ describe(sanitizeSendTransactionInput, () => {
expect(result).toEqual(expected);
});

test('Transforms "TransferWithMemo" transaction input with incorrect type as expected', () => {
const type = AccountTransactionType.TransferWithMemo;
const memo = new DataBlob(Buffer.from('Some memo message'));
const address = AccountAddress.fromBase58(accountAddress);

const expectedPayload: SimpleTransferWithMemoPayload = {
toAddress: AccountAddress.fromBase58(accountAddress),
amount: CcdAmount.fromMicroCcd(amount),
memo,
};

const expected: SanitizedSendTransactionInput = {
accountAddress: AccountAddress.fromBase58(accountAddress),
type,
payload: expectedPayload,
};

const payload: SimpleTransferWithMemoPayload = {
toAddress: {
address: address.address,
decodedAddress: address.decodedAddress,
__type: 'ccd_account_address',
},
amount: {
microCcdAmount: amount,
__type: 'ccd_ccd_amount',
},
memo: {
data: memo.data,
__type: 'ccd_data_blob',
},
} as unknown as SimpleTransferWithMemoPayload;

const result = sanitizeSendTransactionInput(accountAddress, type, payload);
expect(result).toEqual(expected);
const parsed = jsonParse(jsonStringify(result));
expect(() => parsed.payload.memo.toJSON()).not.toThrow();
expect(parsed).toEqual(expected);
});

test('Transforms "ConfigureBaker" transaction input as expected', () => {
const type = AccountTransactionType.ConfigureBaker;
const keys: BakerKeysWithProofs = {
Expand Down Expand Up @@ -501,7 +543,32 @@ describe(sanitizeSendTransactionInput, () => {
};
const result = sanitizeSendTransactionInput(accountAddress, type, payload);
expect(result).toEqual(expected);
expect(result.payload).toBe(payload);
expect((result.payload as RegisterDataPayload).data).toStrictEqual(payload.data);
});

test('Transformed "RegisterData" transaction input with "DataBlob"-like parameter can be parsed', () => {
const type = AccountTransactionType.RegisterData;

const payload: RegisterDataPayload = {
data: {
__type: 'ccd_data_blob',
data: new DataBlob(Buffer.from('This is data!')).data,
} as unknown as DataBlob,
};

const expected: SanitizedSendTransactionInput = {
accountAddress: AccountAddress.fromBase58(accountAddress),
type,
payload: {
data: new DataBlob(Buffer.from('This is data!')),
},
};

const result = sanitizeSendTransactionInput(accountAddress, type, payload);
expect(result).toEqual(expected);
const parsed = jsonParse(jsonStringify(result));
orhoj marked this conversation as resolved.
Show resolved Hide resolved
expect(() => parsed.payload.data.toJSON()).not.toThrow();
expect(parsed).toEqual(expected);
});

test('Transforms "UpdateCredentials" transaction input as expected', () => {
Expand Down
1 change: 1 addition & 0 deletions packages/browser-wallet/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
- Sign message's stringification failing with new `deserializeTypeValue`.
- An issue where the max contract execution energy was not rendered correctly for init contract transactions.
- Updated web-sdk to fix an issue where init contract transactions were not serialized correctly.
- Errors in wallet-api from version mismatch between wallet and dApps.

## 1.1.10

Expand Down
Loading