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

chore: src-01 and sro-01 #1745

Closed
wants to merge 1 commit into from
Closed
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 packages/core/src/transaction/Clause.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,14 @@
type ABIFunction,
type Address,
type HexUInt,
type VTHO

Check failure on line 11 in packages/core/src/transaction/Clause.ts

View workflow job for this annotation

GitHub Actions / install-build / Build & Lint

'VTHO' is defined but never used
} from '../vcdm';
import { Hex } from '../vcdm/Hex';
import { HexInt } from '../vcdm/HexInt';
import type { ClauseOptions } from './ClauseOptions';
import type { DeployParams } from './DeployParams';
import type { TransactionClause } from './TransactionClause';
import { type FungibleToken } from '../vcdm/currency/FungibleToken';

/**
* This class represent a transaction clause.
Expand Down Expand Up @@ -221,7 +222,7 @@
*
* @param {Address} tokenAddress - The address of the VIP180 token.
* @param {Address} recipientAddress - The address of the recipient.
* @param {VTHO} amount - The amount of token to be transferred.
* @param {FungibleToken} amount - The amount of token to be transferred.
* @param {ClauseOptions} [clauseOptions] - Optional clause settings.
* @return {Clause} The clause to transfer VIP180 tokens as part of a transaction.
* @throws {InvalidDataType} Throws an error if the amount is not a positive integer.
Expand All @@ -231,7 +232,7 @@
public static transferToken(
tokenAddress: Address,
recipientAddress: Address,
amount: VTHO,
amount: FungibleToken,
clauseOptions?: ClauseOptions
): Clause {
if (amount.value.isFinite() && amount.value.isPositive()) {
Expand Down
39 changes: 39 additions & 0 deletions packages/core/src/vcdm/currency/FungibleToken.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
import { type FixedPointNumber } from '../FixedPointNumber';
import { type Txt } from '../Txt';
import { Coin } from './Coin';

/**
* Represents a fungible token monetary amount.
*
* @extends Coin
*/
class FungibleToken extends Coin {
/**
* Wei default decimals.
*/
private static readonly WEI_DEFAULT = 18n;

/**
* Wei number of fractional digits to express the token with.
*/
private readonly _decimals: bigint;

/**
* Represents this monetary amount in terms of {@link Units.wei}.
*
* @type {bigint}
*/
public readonly wei: bigint;

protected constructor(
symbol: Txt,
value: FixedPointNumber,
decimals: bigint = FungibleToken.WEI_DEFAULT
) {
super(symbol, value);
this._decimals = decimals;
this.wei = value.dp(this._decimals).scaledValue;
}
}

export { FungibleToken };
21 changes: 8 additions & 13 deletions packages/core/src/vcdm/currency/VTHO.ts
Original file line number Diff line number Diff line change
@@ -1,16 +1,16 @@
import { Coin } from './Coin';
import { FixedPointNumber } from '../FixedPointNumber';
import { Txt } from '../Txt';
import { Units } from './Units';
import { FungibleToken } from './FungibleToken';

Copy link

Choose a reason for hiding this comment

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

💡 Codebase verification

The review comment is valid and the suggested fix is necessary
The Units enum defines valid values from 0 to 18, but there's no runtime validation to prevent invalid values that could cause the exponent calculation to fail. The suggested range check aligns with the codebase's unit system and prevents potential runtime errors.

  • The fix should be applied to the of method in packages/core/src/vcdm/currency/VTHO.ts
  • Additional test cases should be added to packages/core/tests/vcdm/currency/VTHO.unit.test.ts to verify the validation
🔗 Analysis chain

Validate unit range for the of method.
When choosing a Units value that is significantly larger than VTHO_DECIMALS, (VTHO.VTHO_DECIMALS - BigInt(unit)) could become negative, potentially risking incorrect or unexpected scaling. Add a check to ensure unit remains in a valid range.

public static of(
    value: bigint | number | string | FixedPointNumber,
    unit: Units = Units.ether
): VTHO {
    const fpn =
        value instanceof FixedPointNumber
            ? value
            : FixedPointNumber.of(value);

+   // Simple range check if unit is larger than 18 or negative.
+   if (BigInt(unit) > VTHO.VTHO_DECIMALS || BigInt(unit) < 0n) {
+       throw new Error('Invalid unit range for VTHO calculation');
+   }

    return new VTHO(
        fpn.div(
            FixedPointNumber.of(10n ** (VTHO.VTHO_DECIMALS - BigInt(unit)))
        )
    );
}

Also applies to: 11-11, 13-13, 26-26, 34-34, 56-58

🏁 Scripts executed

The following scripts were executed for the analysis:

Error: No shell scripts found


Script:

#!/bin/bash
# Get the complete VTHO implementation and related token files
echo "=== VTHO.ts ==="
cat packages/core/src/vcdm/currency/VTHO.ts

echo -e "\n=== Looking for similar token implementations ==="
fd ".*Token.*\.ts$" packages/core/src/vcdm/currency

echo -e "\n=== Searching for VTHO unit tests ==="
fd ".*VTHO.*test.*" packages/core

Length of output: 2516


Script:

#!/bin/bash
# Check the base class, Units enum, and tests
echo "=== FungibleToken.ts ==="
cat packages/core/src/vcdm/currency/FungibleToken.ts

echo -e "\n=== Units.ts ==="
cat packages/core/src/vcdm/currency/Units.ts

echo -e "\n=== VTHO.unit.test.ts ==="
cat packages/core/tests/vcdm/currency/VTHO.unit.test.ts

Length of output: 7040

/**
* Represents a
* [VeChain VeThor](https://docs.vechain.org/introduction-to-vechain/dual-token-economic-model/vethor-vtho)
* monetary amount.
*
* @extends Coin
* @extends FungibleToken
*/
class VTHO extends Coin {
class VTHO extends FungibleToken {
/**
* The code for VET is the sequence of Unicode
* - U+1D64D - mathematical double strike capital letter 'V',
Expand All @@ -23,22 +23,15 @@ class VTHO extends Coin {
/**
* Wei fractional digits to express this value.
*/
private static readonly WEI_FD = 18n;

/**
* Represents this monetary amount in terms of {@link Units.wei}.
*
* @type {bigint}
*/
public readonly wei: bigint = this.value.dp(VTHO.WEI_FD).scaledValue;
private static readonly VTHO_DECIMALS = 18n;

/**
* Create a new instance with the given `value`.
*
* @param {FixedPointNumber} value The value to be used for initializing the instance.
*/
protected constructor(value: FixedPointNumber) {
super(VTHO.CODE, value);
super(VTHO.CODE, value, VTHO.VTHO_DECIMALS);
}

/**
Expand All @@ -60,7 +53,9 @@ class VTHO extends Coin {
? value
: FixedPointNumber.of(value);
return new VTHO(
fpn.div(FixedPointNumber.of(10n ** (VTHO.WEI_FD - BigInt(unit))))
fpn.div(
FixedPointNumber.of(10n ** (VTHO.VTHO_DECIMALS - BigInt(unit)))
)
);
}
}
Expand Down
47 changes: 47 additions & 0 deletions packages/core/tests/vcdm/currency/FungibleToken.unit.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
import { expect } from '@jest/globals';
import { FixedPointNumber, Txt } from '../../../src';
import { FungibleToken } from '../../../src/vcdm/currency/FungibleToken';

/**
* Test FungibleToken class.
* @group unit/vcdm
*/

const tokenValue = '123456789.012345678';
const tokenDecimals = 20n; // 18 decimals is tested via VTHO

class TestToken extends FungibleToken {
constructor(value: FixedPointNumber) {
super(Txt.of('TEST'), value, tokenDecimals);
}
}

class DefaultDecimalsToken extends FungibleToken {
constructor(value: FixedPointNumber) {
super(Txt.of('DEFAULT'), value);
}
}

describe('FungibleToken tests', () => {
test('toString method', () => {
const token = new TestToken(FixedPointNumber.of(tokenValue));
const expected = `${Txt.of(tokenValue)} ${token.code}`;
const actual = token.toString();
expect(actual).toEqual(expected);
});

test('Wei value', () => {
const expectedWei =
FixedPointNumber.of(tokenValue).dp(tokenDecimals).scaledValue;
const token = new TestToken(FixedPointNumber.of(tokenValue));
const actualWei = token.wei;
expect(actualWei).toEqual(expectedWei);
});

test('Default decimals', () => {
const expectedWei = FixedPointNumber.of(tokenValue).dp(18n).scaledValue;
const token = new DefaultDecimalsToken(FixedPointNumber.of(tokenValue));
const actualWei = token.wei;
expect(actualWei).toEqual(expectedWei);
});
});
8 changes: 8 additions & 0 deletions packages/core/tests/vcdm/currency/VTHO.unit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,4 +24,12 @@ describe('VTHO class tests', () => {
).toString();
expect(actual).toEqual(expected);
});

test('Wei value', () => {
const expected = FixedPointNumber.of(VTHOFixture.value).dp(
18n
).scaledValue;
const actual = VTHO.of(FixedPointNumber.of(VTHOFixture.value)).wei;
expect(actual).toEqual(expected);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ class HardhatVeChainProvider extends VeChainProvider {
}

/**
* Overload off the send method
* Overload of the send method
*
* @param method - The method to call.
* @param params - The parameters to pass to the method.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -195,9 +195,9 @@ abstract class VeChainAbstractSigner implements VeChainSigner {

/**
* Estimates the required gas required to execute //tx// on the Blockchain. This
* will be the expected amount a transaction will require
* will be the expected amount a transaction will need
* to successfully run all the necessary computations and store the needed state
* that the transaction intends.
* for the transaction.
*
* @param transactionToEstimate - The transaction to estimate gas for
* @returns the total estimated gas required
Expand Down
Loading