Skip to content

Commit

Permalink
fix: Remove redundant assertions (#313)
Browse files Browse the repository at this point in the history
* fix: remove assertions.ts form index.ts

* feat: assert for array

* feat: assert for hdnode.ts

* fix: missing

* feat: common assertion for secp256k1.ts and some typo fix

* feat: common assertion for transaction sign handler

* fix: missing tsdoc

* feat: common assertion for transaction

* feat: common assertion for poll

* feat: common assertion for transaction thorest client

* feat: common assertion for accounts thorest client

* feat: contrib guidelines for common assertions

* fix: typo

* fix: remove assertion wrong
  • Loading branch information
rodolfopietro97 authored Nov 29, 2023
1 parent e9772f8 commit e2ae986
Show file tree
Hide file tree
Showing 26 changed files with 357 additions and 251 deletions.
5 changes: 5 additions & 0 deletions .github/CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,11 @@ function some_function(input: any) {
}
```

### Common assertions
It is often observed that certain assertions are applicable across various contexts.
Adhering to the principle of Don't Repeat Yourself (DRY), it is imperative that these assertions be consolidated in a universally accessible file.
This file shall be designated as `helpers/assertions.ts` and should be referenced by each module requiring its contents.

# Code of Conduct

We are committed to providing a welcoming and inclusive environment for everyone who contributes to or interacts with the vechain SDK project. To ensure a positive experience for our community, we have established a [Code of Conduct](CODE_OF_CONDUCT.md) that outlines our expectations for behavior. We encourage all contributors, maintainers, and users to familiarize themselves with this code, as it reflects our commitment to creating a diverse and respectful community. Thank you for helping us maintain a welcoming and collaborative space for all.
Expand Down
30 changes: 28 additions & 2 deletions packages/core/src/encoding/rlp/helpers/assertions.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
import { assert, RLP } from '@vechainfoundation/vechain-sdk-errors';
import {
assert,
RLP as RLPError,
RLP
} from '@vechainfoundation/vechain-sdk-errors';
import { type RLPInput } from '../types';

/**
Expand All @@ -19,4 +23,26 @@ function assertIsValidBuffer(
});
}

export { assertIsValidBuffer };
/**
* Asserts that the data is an array.
*
* @param arrayToCheck - The arrayToCheck to validate.
* @param context - Descriptive context for error messages.
*
* @throws{InvalidRLPError}
*/
function assertIsArray<ArrayType>(
arrayToCheck: ArrayType,
context: string
): void {
assert(
Array.isArray(arrayToCheck),
RLPError.INVALID_RLP,
'expected array',
{
context
}
);
}

export { assertIsValidBuffer, assertIsArray };
1 change: 0 additions & 1 deletion packages/core/src/encoding/rlp/helpers/index.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
export * from './assertions';
export * from './numerickind';
export * from './hexblobkind';
export * from './fixedhexblobkind';
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/encoding/rlp/kind/bufferkind.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { ScalarKind } from './scalarkind.abstract';
import { type BufferOutput, type DataOutput, type RLPInput } from '../types';
import { assertIsValidBuffer } from '../helpers';
import { assertIsValidBuffer } from '../helpers/assertions';

/**
* Asserts that the data is a buffer.
Expand Down
10 changes: 4 additions & 6 deletions packages/core/src/encoding/rlp/rlp.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
} from './types';
import { RLP } from '.';
import { RLP as RLPError, assert } from '@vechainfoundation/vechain-sdk-errors';
import { assertIsArray } from './helpers/assertions';

/**
* Encodes data using the Ethereumjs RLP library.
Expand Down Expand Up @@ -93,9 +94,7 @@ const _packData = (
}

// Valid RLP array
assert(Array.isArray(obj), RLPError.INVALID_RLP, 'expected array', {
context
});
assertIsArray(obj, context);

// ArrayKind: recursively pack each array item based on the shared item profile.
if ('item' in kind && Array.isArray(obj)) {
Expand Down Expand Up @@ -166,9 +165,8 @@ const _unpackData = (
);
}

assert(Array.isArray(packed), RLPError.INVALID_RLP, 'expected array', {
context
});
// Valid RLP array
assertIsArray(packed, context);

// ArrayKind: Recursively unpack each array item based on the shared item profile.
if ('item' in kind && Array.isArray(packed)) {
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/hash/blake2b256.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import blake from 'blakejs';
import { type HashInput, type ReturnType } from './types';
import { assertIsValidReturnType } from './helpers';
import { assertIsValidReturnType } from './helpers/assertions';

/**
* Internal function to compute the blake2b256 256-bit hash of the given data.
Expand Down
13 changes: 12 additions & 1 deletion packages/core/src/hash/helpers/assertions.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,17 @@
import { assert, DATA } from '@vechainfoundation/vechain-sdk-errors';
import { type ReturnType } from '../types';
import { isValidReturnType } from './hashhelpers';

/**
* Checks if the return type is valid
*
* Helper method needed to validate the return type of the hash function
*
* @param value - The return type
* @returns A boolean indicating whether the return type is valid
*/
const isValidReturnType = (value: string): boolean => {
return !(value !== 'buffer' && value !== 'hex');
};

/**
* Asserts that the return type of hash is valid.
Expand Down
13 changes: 0 additions & 13 deletions packages/core/src/hash/helpers/hashhelpers.ts

This file was deleted.

2 changes: 0 additions & 2 deletions packages/core/src/hash/helpers/index.ts

This file was deleted.

1 change: 0 additions & 1 deletion packages/core/src/hash/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,3 @@ export * from './types.d';
export * from './blake2b256';
export * from './sha256';
export * from './keccak256';
export * from './helpers';
2 changes: 1 addition & 1 deletion packages/core/src/hash/keccak256.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { ethers } from 'ethers';
import { type ReturnType, type HashInput } from './types';
import { assertIsValidReturnType } from './helpers';
import { assertIsValidReturnType } from './helpers/assertions';

/* --- Overloaded functions start --- */

Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/hash/sha256.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { ethers } from 'ethers';
import { type ReturnType, type HashInput } from './types';
import { assertIsValidReturnType } from './helpers';
import { assertIsValidReturnType } from './helpers/assertions';

/* --- Overloaded functions start --- */

Expand Down
33 changes: 8 additions & 25 deletions packages/core/src/hdnode/hdnode.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import { ethers } from 'ethers';
import {
isDerivationPathValid,
MNEMONIC_WORDLIST_ALLOWED_SIZES,
VET_DERIVATION_PATH,
X_PRIV_PREFIX,
Expand All @@ -12,6 +11,10 @@ import { sha256 } from '../hash';
import { secp256k1 } from '../secp256k1';
import { type WordlistSizeType } from '../mnemonic';
import { assert, HDNODE } from '@vechainfoundation/vechain-sdk-errors';
import {
assertIsValidHdNodeChainCode,
assertIsValidHdNodeDerivationPath
} from './helpers/assertions';

/**
* Generates an HDNode instance using mnemonic words.
Expand All @@ -33,12 +36,7 @@ function fromMnemonic(words: string[], path = VET_DERIVATION_PATH): IHDNode {
);

// Invalid derivation path
assert(
isDerivationPathValid(path),
HDNODE.INVALID_HDNODE_DERIVATION_PATH,
'Invalid derivation path.',
{ path }
);
assertIsValidHdNodeDerivationPath(path);

// normalize words to lowercase
const joinedWords = words.join(' ').toLowerCase();
Expand Down Expand Up @@ -67,12 +65,7 @@ function fromPublicKey(publicKey: Buffer, chainCode: Buffer): IHDNode {
);

// Invalid chain code
assert(
chainCode.length === 32,
HDNODE.INVALID_HDNODE_CHAIN_CODE,
'Invalid chain code. Length must be 32 bytes.',
{ chainCode }
);
assertIsValidHdNodeChainCode(chainCode);

const compressed = secp256k1.extendedPublicKeyToArray(publicKey, true);
const key = Buffer.concat([
Expand Down Expand Up @@ -107,12 +100,7 @@ function fromPrivateKey(privateKey: Buffer, chainCode: Buffer): IHDNode {
);

// Invalid chain code
assert(
chainCode.length === 32,
HDNODE.INVALID_HDNODE_CHAIN_CODE,
'Invalid chain code. Length must be 32 bytes.',
{ chainCode }
);
assertIsValidHdNodeChainCode(chainCode);

const key = Buffer.concat([
X_PRIV_PREFIX,
Expand Down Expand Up @@ -166,12 +154,7 @@ function ethersNodeToOurHDNode(ethersNode: ethers.HDNodeWallet): IHDNode {
},
derivePath(path: string) {
// Invalid derivation path
assert(
isDerivationPathValid(path),
HDNODE.INVALID_HDNODE_DERIVATION_PATH,
'Invalid derivation path given as input.',
{ path }
);
assertIsValidHdNodeDerivationPath(path);

return ethersNodeToOurHDNode(ethersNode.derivePath(path));
}
Expand Down
32 changes: 32 additions & 0 deletions packages/core/src/hdnode/helpers/assertions.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
import { assert, HDNODE } from '@vechainfoundation/vechain-sdk-errors';
import { isDerivationPathValid } from '../../utils';

/**
* Asserts that the derivation path is valid.
*
* @param path - The derivation path to validate.
*/
function assertIsValidHdNodeDerivationPath(path: string): void {
assert(
isDerivationPathValid(path),
HDNODE.INVALID_HDNODE_DERIVATION_PATH,
'Invalid derivation path.',
{ path }
);
}

/**
* Asserts that the chain code is valid.
*
* @param chainCode - The chain code to validate.
*/
function assertIsValidHdNodeChainCode(chainCode: Buffer): void {
assert(
chainCode.length === 32,
HDNODE.INVALID_HDNODE_CHAIN_CODE,
'Invalid chain code. Length must be 32 bytes.',
{ chainCode }
);
}

export { assertIsValidHdNodeDerivationPath, assertIsValidHdNodeChainCode };
39 changes: 39 additions & 0 deletions packages/core/src/keystore/helpers/assertions.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
import { assert, SECP256K1 } from '@vechainfoundation/vechain-sdk-errors';

/**
* Assert if private key is valid
*
* @param privateKey - Private key to assert
* @param isValidPrivateKeyFunction - Function to assert private key
*/
function assertIsValidPrivateKey(
privateKey: Buffer,
isValidPrivateKeyFunction: (privateKey: Buffer) => boolean
): void {
assert(
isValidPrivateKeyFunction(privateKey),
SECP256K1.INVALID_SECP256k1_PRIVATE_KEY,
'Invalid private key given as input. Length must be 32 bytes',
{ privateKey }
);
}

/**
* Assert if message hash is valid
*
* @param msgHash - Message hash to assert
* @param isValidMessageHashFunction - Function to assert message hash
*/
function assertIsValidSecp256k1MessageHash(
msgHash: Buffer,
isValidMessageHashFunction: (messageHash: Buffer) => boolean
): void {
assert(
isValidMessageHashFunction(msgHash),
SECP256K1.INVALID_SECP256k1_MESSAGE_HASH,
'Invalid message hash given as input. Length must be 32 bytes',
{ msgHash }
);
}

export { assertIsValidPrivateKey, assertIsValidSecp256k1MessageHash };
Loading

1 comment on commit e2ae986

@github-actions
Copy link

Choose a reason for hiding this comment

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

Test Coverage

Summary

Lines Statements Branches Functions
Coverage: 100%
100% (1202/1202) 100% (267/267) 100% (252/252)
Title Tests Skipped Failures Errors Time
core 319 0 💤 0 ❌ 0 🔥 1m 32s ⏱️
network 80 0 💤 0 ❌ 0 🔥 53.377s ⏱️
errors 30 0 💤 0 ❌ 0 🔥 10.719s ⏱️

Please sign in to comment.