Skip to content

build_assertion error case test #1463

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.

Sign up for GitHub

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

Merged
merged 26 commits into from
Mar 17, 2023
Merged

build_assertion error case test #1463

Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
924deee
error verify
0xverin Mar 7, 2023
8e5484a
batch call of identity
0xverin Mar 8, 2023
0818632
batch call of vc
0xverin Mar 8, 2023
658cb9b
format code
0xverin Mar 8, 2023
7eafecf
delete useless code
0xverin Mar 8, 2023
820ac8a
verify error identity
0xverin Mar 8, 2023
6be62b8
create error identities
0xverin Mar 8, 2023
c46db24
delete useless code
0xverin Mar 8, 2023
8013596
delete useless code
0xverin Mar 8, 2023
8e55e0d
add comment&delete useless code
0xverin Mar 8, 2023
363bad8
format code
0xverin Mar 8, 2023
8c5da1c
txs types
0xverin Mar 8, 2023
e092369
step error vc
0xverin Mar 13, 2023
11f48cd
catch dispatchError
0xverin Mar 15, 2023
48fd295
Merge branch 'dev' into 1392-build-assertion
0xverin Mar 15, 2023
8b4f9d1
revoke error test
0xverin Mar 15, 2023
f70a3fe
Merge branch 'dev' into 1392-build-assertion
0xverin Mar 15, 2023
9f1079c
requestVc error
0xverin Mar 15, 2023
25eeedf
change assert log
0xverin Mar 15, 2023
30e0998
fix merge errors
0xverin Mar 15, 2023
22cfc31
add checkFailReason
0xverin Mar 16, 2023
f044ffa
check fail reason
0xverin Mar 16, 2023
c541348
move nonce out loop
0xverin Mar 16, 2023
19145ac
format code
0xverin Mar 16, 2023
9e77c3b
change test flow(only use A1 trigger error event)
0xverin Mar 16, 2023
2cfc52f
format code
0xverin Mar 17, 2023
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
56 changes: 46 additions & 10 deletions tee-worker/ts-tests/identity.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,10 @@
import { describeLitentry, encryptWithTeeShieldingKey, generateVerificationMessage, listenEvent, sendTxUntilInBlock } from './utils';
import {
describeLitentry,
encryptWithTeeShieldingKey,
generateVerificationMessage,
listenEvent,
sendTxUntilInBlock,
} from './utils';
import { hexToU8a, u8aConcat, u8aToHex, u8aToU8a, stringToU8a } from '@polkadot/util';
import {
setUserShieldingKey,
Expand Down Expand Up @@ -213,7 +219,6 @@ describeLitentry('Test Identity', (context) => {
assert.isNotEmpty(resp_extension_substrate.challengeCode, 'challengeCode empty');
}
});

step('verify identities', async function () {
//Alice verify all identities
const [twitter_identity_verified, ethereum_identity_verified, substrate_identity_verified] =
Expand Down Expand Up @@ -310,7 +315,9 @@ describeLitentry('Test Identity', (context) => {

step('remove prime identity NOT allowed', async function () {
// create substrate identity
const [resp_substrate] = (await createIdentities(context, context.defaultSigner[0], aesKey, true, [substrateIdentity])) as IdentityGenericEvent[];
const [resp_substrate] = (await createIdentities(context, context.defaultSigner[0], aesKey, true, [
substrateIdentity,
])) as IdentityGenericEvent[];
Copy link
Collaborator

@Kailai-Wang Kailai-Wang Mar 15, 2023

Choose a reason for hiding this comment

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

please feel free to adjust .prettierrc if you believe the old one has better readability

assertIdentityCreated(context.defaultSigner[0], resp_substrate);

if (resp_substrate) {
Expand All @@ -330,13 +337,9 @@ describeLitentry('Test Identity', (context) => {
}

// remove substrate identity
const [substrate_identity_removed] = (await removeIdentities(
context,
context.defaultSigner[0],
aesKey,
true,
[substrateIdentity]
)) as IdentityGenericEvent[];
const [substrate_identity_removed] = (await removeIdentities(context, context.defaultSigner[0], aesKey, true, [
substrateIdentity,
])) as IdentityGenericEvent[];
assertIdentityRemoved(context.defaultSigner[0], substrate_identity_removed);

// remove prime identity
Expand All @@ -353,6 +356,7 @@ describeLitentry('Test Identity', (context) => {
await sendTxUntilInBlock(context.substrate, tx, context.defaultSigner[0]);

const events = await listenEvent(context.substrate, 'identityManagement', ['StfError']);

expect(events.length).to.be.equal(1);
const result = events[0].method as string;
});
Expand Down Expand Up @@ -389,6 +393,38 @@ describeLitentry('Test Identity', (context) => {
});
});

step('remove error identities', async function () {
//remove a nonexistent identity from an account
const resp_not_exist_identities = (await removeErrorIdentities(context, context.defaultSigner[0], true, [
twitterIdentity,
ethereumIdentity,
substrateIdentity,
])) as string[];

resp_not_exist_identities.map((item: any) => {
const result = item.toHuman().data.reason;
assert(
result.search('IdentityNotExist') !== -1,
'remove twitter should fail with reason `IdentityNotExist`'
);
});

//remove a challenge code before the code is set
const resp_not_created_identities = (await removeErrorIdentities(context, context.defaultSigner[2], true, [
twitterIdentity,
ethereumIdentity,
substrateIdentity,
])) as string[];

resp_not_created_identities.map((item: any) => {
const result = item.toHuman().data.reason;
assert(
result.search('IdentityNotExist') !== -1,
'remove twitter should fail with reason `IdentityNotExist`'
);
});
});

Copy link
Collaborator

@Kailai-Wang Kailai-Wang Mar 15, 2023

Choose a reason for hiding this comment

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

  1. This step seems identical with the previous one, we should have comment at least about why we repeat it
  2. even though we repeat it - we should have a different step name
  3. we should avoid duplicate code on step-level
  4. we should avoid duplicate code inside a step: I don't see any difference between const resp_not_exist_identities... and const resp_not_created_identities... except for the signer. Can't we use a function to wrap it up?

Copy link
Contributor Author

@0xverin 0xverin Mar 15, 2023

Choose a reason for hiding this comment

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

The difference between the two removeErrorIdentities is that there is no challengeCode for context.defaultSigner[2],so for the convenience of testing, I directly change the signer and repeat it.

Copy link
Collaborator

@Kailai-Wang Kailai-Wang Mar 15, 2023

Choose a reason for hiding this comment

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

Yes I know the signer is different :) I mean we should point it out in the comment and try to remove duplicate code.

step('set error user shielding key', async function () {
const result = await setErrorUserShieldingKey(context, context.defaultSigner[0], errorAseKey, true);
assert.equal(
Expand Down
77 changes: 77 additions & 0 deletions tee-worker/ts-tests/indirect_error_calls.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { encryptWithTeeShieldingKey, listenEvent, sendTxUntilInBlock, sendTxUnti
import { KeyringPair } from '@polkadot/keyring/types';
import { HexString } from '@polkadot/util/types';
import {
Assertion,
IntegrationTestContext,
LitentryIdentity,
LitentryValidationData,
Expand Down Expand Up @@ -149,3 +150,79 @@ export async function removeErrorIdentities(
}
return undefined;
}
export async function requestErrorVCs(
context: IntegrationTestContext,
signer: KeyringPair,
aesKey: HexString,
listening: boolean,
mrEnclave: HexString,
assertion: Assertion
): Promise<
| {
account: HexString;
index: HexString;
vc: HexString;
}[]
| undefined
> {
let txs: TransactionSubmit[] = [];
let len = 0;

for (const key in assertion) {
len++;
const tx = context.substrate.tx.vcManagement.requestVc(mrEnclave, {
[key]: assertion[key as keyof Assertion],
});
const nonce = await context.substrate.rpc.system.accountNextIndex(signer.address);

let newNonce = nonce.toNumber() + (len - 1);
txs.push({ tx, nonce: newNonce });
}

await sendTxUntilInBlockList(context.substrate, txs, signer);

if (listening) {
const events = (await listenEvent(context.substrate, 'vcManagement', ['StfError'])) as any;
expect(events.length).to.be.equal(len);
return events;
}
return undefined;
}
export async function disableErrorVCs(
context: IntegrationTestContext,
signer: KeyringPair,
listening: boolean,
indexList: HexString[]
): Promise<string[] | undefined> {
let txs: TransactionSubmit[] = [];

for (let k = 0; k < indexList.length; k++) {
const tx = context.substrate.tx.vcManagement.disableVc(indexList[k]);
const nonce = await context.substrate.rpc.system.accountNextIndex(signer.address);
Copy link
Collaborator

@Kailai-Wang Kailai-Wang Mar 15, 2023

Choose a reason for hiding this comment

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

see below

let newNonce = nonce.toNumber() + k;
txs.push({ tx, nonce: newNonce });
}

const res = (await sendTxUntilInBlockList(context.substrate, txs, signer)) as string[];

return res.length ? res : undefined;
}
export async function revokeErrorVCs(
context: IntegrationTestContext,
signer: KeyringPair,
listening: boolean,
indexList: HexString[]
): Promise<string[] | undefined> {
let txs: TransactionSubmit[] = [];

for (let k = 0; k < indexList.length; k++) {
const tx = context.substrate.tx.vcManagement.revokeVc(indexList[k]);
const nonce = await context.substrate.rpc.system.accountNextIndex(signer.address);
Copy link
Collaborator

@Kailai-Wang Kailai-Wang Mar 15, 2023

Choose a reason for hiding this comment

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

A small improvement: I think we can move it out of the loop - we'll get the same result each time anyway.

let newNonce = nonce.toNumber() + k;
txs.push({ tx, nonce: newNonce });
}

const res = (await sendTxUntilInBlockList(context.substrate, txs, signer)) as string[];

return res.length ? res : undefined;
}
51 changes: 30 additions & 21 deletions tee-worker/ts-tests/utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import {
teeTypes,
WorkerRpcReturnValue,
TransactionSubmit,
JsonSchema,
} from './type-definitions';
import { blake2AsHex, cryptoWaitReady } from '@polkadot/util-crypto';
import { ApiTypes, SubmittableExtrinsic } from '@polkadot/api/types';
Expand All @@ -27,7 +26,6 @@ import { ethers } from 'ethers';
import { generateTestKeys } from './web3/functions';
import { expect } from 'chai';
import { Base64 } from 'js-base64';
import Ajv from 'ajv';
import * as ed from '@noble/ed25519';
const base58 = require('micro-base58');
const crypto = require('crypto');
Expand Down Expand Up @@ -121,25 +119,37 @@ export async function sendTxUntilInBlock(api: ApiPromise, tx: SubmittableExtrins
}

export async function sendTxUntilInBlockList(api: ApiPromise, txs: TransactionSubmit[], signer: KeyringPair) {
return new Promise<{
block: string;
}>(async (resolve, reject) => {
await Promise.all(
txs.map(async ({ tx, nonce }) => {
// await tx.paymentInfo(signer);
return Promise.all(
txs.map(async ({ tx, nonce }) => {
const result = await new Promise((resolve, reject) => {
tx.signAndSend(signer, { nonce }, (result) => {
if (result.status.isInBlock) {
console.log(`Transaction included at blockHash ${result.status.asInBlock}`);
resolve({
block: result.status.asInBlock.toString(),
});
//catch error
if (result.dispatchError) {
if (result.dispatchError.isModule) {
const decoded = api.registry.findMetaError(result.dispatchError.asModule);
const { docs, name, section } = decoded;

console.log(`${section}.${name}: ${docs.join(' ')}`);
resolve(`${section}.${name}`);
} else {
console.log(result.dispatchError.toString());
resolve(result.dispatchError.toString());
}
} else {
console.log(`Transaction included at blockHash ${result.status.asInBlock}`);
resolve({
block: result.status.asInBlock.toString(),
});
}
} else if (result.status.isInvalid) {
reject(`Transaction is ${result.status}`);
}
});
})
);
});
});
return result;
})
);
}

// Subscribe to the chain until we get the first specified event with given `section` and `methods`.
Expand Down Expand Up @@ -362,12 +372,11 @@ export async function checkVc(vcObj: any, index: HexString, proof: any, api: Api

//Check VC json fields
export async function checkJSON(vc: any, proofJson: any): Promise<boolean> {
//check JsonSchema
const ajv = new Ajv();
const validate = ajv.compile(JsonSchema);
const isValid = validate(vc);
expect(isValid).to.be.true;

const vcStatus = ['@context', 'type', 'credentialSubject', 'issuer'].every(
(key) =>
vc.hasOwnProperty(key) && (vc[key] != '{}' || vc[key] !== '[]' || vc[key] !== null || vc[key] !== undefined)
);
expect(vcStatus).to.be.true;
expect(
vc.type[0] === 'VerifiableCredential' &&
vc.issuer.id === proofJson.verificationMethod &&
Expand Down
44 changes: 44 additions & 0 deletions tee-worker/ts-tests/vc.test.ts
View file
Open in desktop
Copy link
Collaborator

@Kailai-Wang Kailai-Wang Mar 15, 2023

Choose a reason for hiding this comment

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

One of my general feelings here is (you might have other opinions):

We want to cover as many test cases as possible, but we don't want to over-test them. For example, we want to test the error handling of the case where a user requests a VC without setting the shielding key. This is a good test case, but we don't have to repeat it for all VC types that we have - because there isn't any difference in terms of such error handling for different assertion enums (if we have one someday, then we should test it).

So it doesn't make much difference you test A1 only vs you test A1 - A11, one VC type is enough. Making it a vector of VCs impairs the code readability and prolongs the whole test time unnecessarily.

The same applies to the identity test.

Copy link
Contributor Author

@0xverin 0xverin Mar 15, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

My idea is to select a random assertion to trigger the specified error, I don't know the difference between them, I worry that it will not cover all case(maybe A1 trigger,but A2 not trigger)

Copy link
Collaborator

@Kailai-Wang Kailai-Wang Mar 15, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

select a random assertion

But if I understand the code correctly, we iterate over all assertions here (instead of choosing one randomly). And yes there's no difference in terms of that specific error handling, we can ask eric or zhouhui if we are unsure.

I'm mainly concerned about the execution time - it probably won't be such a problem if a step takes 5s. Imagine we have 100 assertions later, we can't afford to test them all for a generic handling that applies to all assertion types (we should still do individual test if something differs)

Copy link
Contributor Author

@0xverin 0xverin Mar 16, 2023

Choose a reason for hiding this comment

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

I have confirmed with @zhouhuitian that only one test is needed, and all the procedures are the same,so only use A1 to triggers the error events.

Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { assert } from 'chai';
import { u8aToHex } from '@polkadot/util';
import { HexString } from '@polkadot/util/types';
import { blake2AsHex } from '@polkadot/util-crypto';
import { requestErrorVCs, disableErrorVCs, revokeErrorVCs } from './indirect_error_calls';

const assertion = <Assertion>{
A1: 'A1',
Expand Down Expand Up @@ -42,6 +43,7 @@ describeLitentry('VC test', async (context) => {
for (let k = 0; k < res.length; k++) {
const vcString = res[k].vc.replace('0x', '');
const vcObj = JSON.parse(vcString);
console.log('---------VC json----------', vcObj);

const vcProof = vcObj.proof;

Expand All @@ -57,7 +59,25 @@ describeLitentry('VC test', async (context) => {
indexList.push(res[k].index);
}
});
step('Request Error VC', async () => {
const resp_request_error = (await requestErrorVCs(
context,
context.defaultSigner[1],
aesKey,
true,
context.mrEnclave,
assertion
)) as any;

resp_request_error.map((item: any) => {
const result = item.toHuman().data.reason;

assert(
result.search('User shielding key is missing') !== -1,
'requestVc should fail with reason `User shielding key is missing`'
);
});
});
step('Disable VC', async () => {
const res = (await disableVCs(context, context.defaultSigner[0], aesKey, true, indexList)) as HexString[];
for (let k = 0; k < res.length; k++) {
Expand All @@ -66,6 +86,18 @@ describeLitentry('VC test', async (context) => {
assert.equal(registry.toHuman()!['status'], 'Disabled');
}
});
step('Disable error VC', async () => {
//Bob dont't request VC before
const resp_disable_error = (await disableErrorVCs(
context,
context.defaultSigner[0],
true,
indexList
)) as HexString[];
for (let k = 0; k < resp_disable_error.length; k++) {
assert.equal(resp_disable_error[k], 'vcManagement.VCAlreadyDisabled', 'check disableVc error');
}
});

step('Revoke VC', async () => {
const res = (await revokeVCs(context, context.defaultSigner[0], aesKey, true, indexList)) as HexString[];
Expand All @@ -75,4 +107,16 @@ describeLitentry('VC test', async (context) => {
assert.equal(registry.toHuman(), null);
}
});

step('Revoke Error VC', async () => {
const resp_revoke_error = (await revokeErrorVCs(
context,
context.defaultSigner[0],
true,
indexList
)) as string[];
for (let k = 0; k < resp_revoke_error.length; k++) {
assert.equal(resp_revoke_error[k], 'vcManagement.VCNotExist', 'check revokeVc error');
}
});
});