Skip to content

Commit

Permalink
Increase Unit test coverage for web3-eth above 90% (#6663)
Browse files Browse the repository at this point in the history 
* fix issues in Common config and add a test to accounts

* add test cases

* update CHAGELOG.md
  • Loading branch information
@Muhammad-Altabba
Muhammad-Altabba authored Jan 3, 2024
1 parent 6b2dbe3 commit 6c075db
Show file tree
Hide file tree
Showing 6 changed files with 440 additions and 79 deletions.
6 changes: 5 additions & 1 deletion packages/web3-eth-accounts/CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -149,4 +149,8 @@ Documentation:

- Fixed `recover` function, `v` will be normalized to value 0,1 (#6344)

## [Unreleased]
## [Unreleased]

### Fixed

- Send Transaction config used to be ignored if the passed `common` did not have a `copy()` and the `chainId` was not provided (#6663)
32 changes: 31 additions & 1 deletion packages/web3-eth-accounts/src/tx/baseTransaction.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ You should have received a copy of the GNU Lesser General Public License
along with web3.js. If not, see <http://www.gnu.org/licenses/>.
*/

import { Numbers } from 'web3-types';
import { Common as CommonType, Numbers } from 'web3-types';
import { bytesToHex } from 'web3-utils';
import { MAX_INTEGER, MAX_UINT64, SECP256K1_ORDER_DIV_2, secp256k1 } from './constants.js';
import { toUint8Array, uint8ArrayToBigInt, unpadUint8Array } from '../common/utils.js';
Expand Down Expand Up @@ -389,6 +389,8 @@ export abstract class BaseTransaction<TransactionObject> {
* @param chainId - Chain ID from tx options (typed txs) or signature (legacy tx)
*/
protected _getCommon(common?: Common, chainId?: Numbers) {
// TODO: this function needs to be reviewed and the code to be more clean
// check issue https://github.com/web3/web3.js/issues/6666
// Chain ID provided
if (chainId !== undefined) {
const chainIdBigInt = uint8ArrayToBigInt(toUint8Array(chainId));
Expand Down Expand Up @@ -425,6 +427,34 @@ export abstract class BaseTransaction<TransactionObject> {
if (common?.copy && typeof common?.copy === 'function') {
return common.copy();
}
// TODO: Recheck this next block when working on https://github.com/web3/web3.js/issues/6666
// This block is to handle when `chainId` was not passed and the `common` object does not have `copy()`
// If it was meant to be unsupported to process `common` in this case, an exception should be thrown instead of the following block
if (common) {
const hardfork =
typeof common.hardfork === 'function'
? common.hardfork()
: // eslint-disable-next-line @typescript-eslint/unbound-method
(common.hardfork as unknown as string);

return Common.custom(
{
name: 'custom-chain',
networkId: common.networkId
? common.networkId()
: BigInt((common as unknown as CommonType).customChain?.networkId) ??
undefined,
chainId: common.chainId
? common.chainId()
: BigInt((common as unknown as CommonType).customChain?.chainId) ??
undefined,
},
{
baseChain: this.DEFAULT_CHAIN,
hardfork: hardfork || this.DEFAULT_HARDFORK,
},
);
}

return new Common({ chain: this.DEFAULT_CHAIN, hardfork: this.DEFAULT_HARDFORK });
}
Expand Down
2 changes: 1 addition & 1 deletion packages/web3-eth/src/utils/prepare_transaction_for_signing.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ const getEthereumjsTransactionOptions = (
if (!hasTransactionSigningOptions) {
// if defaultcommon is specified, use that.
if (web3Context.defaultCommon) {
common = web3Context.defaultCommon;
common = { ...web3Context.defaultCommon };

if (isNullish(common.hardfork))
common.hardfork = transaction.hardfork ?? web3Context.defaultHardfork;
Expand Down
151 changes: 149 additions & 2 deletions packages/web3-eth/test/unit/prepare_transaction_for_signing.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ along with web3.js. If not, see <http://www.gnu.org/licenses/>.
*/

import {
Common,
EthExecutionAPI,
HexString,
Web3NetAPI,
Expand All @@ -33,6 +32,7 @@ import {
AccessListEIP2930Transaction,
FeeMarketEIP1559Transaction,
Transaction,
Hardfork,
} from 'web3-eth-accounts';
import { prepareTransactionForSigning } from '../../src/utils/prepare_transaction_for_signing';
import { validTransactions } from '../fixtures/prepare_transaction_for_signing';
Expand Down Expand Up @@ -70,7 +70,7 @@ describe('prepareTransactionForSigning', () => {

if (isNullish(tx.common)) {
if (options.web3Context.defaultCommon) {
const common = options.web3Context.defaultCommon as unknown as Common;
const common = options.web3Context.defaultCommon;
const chainId = common.customChain.chainId as string;
const networkId = common.customChain.networkId as string;
const name = common.customChain.name as string;
Expand Down Expand Up @@ -102,6 +102,153 @@ describe('prepareTransactionForSigning', () => {
expect(ethereumjsTx.common.chainName()).toBe('test');
});
});

it('should be able to read Hardfork from context.defaultHardfork', async () => {
const context = new Web3Context<EthExecutionAPI>({
provider: new HttpProvider('http://127.0.0.1'),
config: { defaultNetworkId: '0x9' },
});
context.defaultChain = 'mainnet';
context.defaultHardfork = Hardfork.Istanbul;

async function transactionBuilder<ReturnType = TransactionType>(options: {
transaction: TransactionType;
web3Context: Web3Context<EthExecutionAPI & Web3NetAPI>;
privateKey?: HexString | Uint8Array;
fillGasPrice?: boolean;
fillGasLimit?: boolean;
}): Promise<ReturnType> {
const tx = { ...options.transaction };
return tx as unknown as ReturnType;
}

context.transactionBuilder = transactionBuilder;

const ethereumjsTx = await prepareTransactionForSigning(
{
chainId: 1458,
nonce: 1,
gasPrice: BigInt(20000000000),
gas: BigInt(21000),
to: '0xF0109fC8DF283027b6285cc889F5aA624EaC1F55',
from: '0x2c7536E3605D9C16a7a3D7b1898e529396a65c23',
value: '1000000000',
input: '',
networkId: 999,
},
context,
);
expect(ethereumjsTx.common.hardfork()).toBe(Hardfork.Istanbul);
expect(ethereumjsTx.common.networkId().toString()).toBe('999');
});

it('should be able to read Hardfork from context.config.defaultHardfork and context.defaultCommon.hardfork', async () => {
const context = new Web3Context<EthExecutionAPI>({
provider: new HttpProvider('http://127.0.0.1'),
config: { defaultNetworkId: '0x9' },
});
context.defaultChain = 'mainnet';

// if the value here is different from the one in context.defaultCommon.hardfork
// Then an error will be thrown:
// "ConfigHardforkMismatchError: Web3Config hardfork doesnt match in defaultHardfork london and common.hardfork istanbul"
context.config.defaultHardfork = Hardfork.Istanbul;
context.defaultCommon = {
customChain: {
name: 'test',
networkId: 111,
chainId: 1458,
},
hardfork: Hardfork.Istanbul,
baseChain: 'mainnet',
} as any;

async function transactionBuilder<ReturnType = TransactionType>(options: {
transaction: TransactionType;
web3Context: Web3Context<EthExecutionAPI & Web3NetAPI>;
privateKey?: HexString | Uint8Array;
fillGasPrice?: boolean;
fillGasLimit?: boolean;
}): Promise<ReturnType> {
const tx = { ...options.transaction };
return tx as unknown as ReturnType;
}

context.transactionBuilder = transactionBuilder;

const ethereumjsTx = await prepareTransactionForSigning(
{
chainId: 1458,
nonce: 1,
gasPrice: BigInt(20000000000),
gas: BigInt(21000),
to: '0xF0109fC8DF283027b6285cc889F5aA624EaC1F55',
from: '0x2c7536E3605D9C16a7a3D7b1898e529396a65c23',
value: '1000000000',
input: '',
},
context,
);
expect(ethereumjsTx.common.hardfork()).toBe(Hardfork.Istanbul);
expect(ethereumjsTx.common.networkId().toString()).toBe('111');
});

it('should give priorities to tx.hardfork and tx.networkId over values from context', async () => {
const context = new Web3Context<EthExecutionAPI>({
provider: new HttpProvider('http://127.0.0.1'),
config: { defaultNetworkId: '0x9' },
});
context.defaultChain = 'mainnet';

// if the value here is different from the one in context.defaultCommon.hardfork
// Then an error will be thrown:
// "ConfigHardforkMismatchError: Web3Config hardfork doesnt match in defaultHardfork london and common.hardfork istanbul"
context.config.defaultHardfork = Hardfork.Istanbul;
context.defaultCommon = {
customChain: {
name: 'test',
networkId: 111,
chainId: 1458,
},
hardfork: Hardfork.Istanbul,
baseChain: 'mainnet',
} as any;

async function transactionBuilder<ReturnType = TransactionType>(options: {
transaction: TransactionType;
web3Context: Web3Context<EthExecutionAPI & Web3NetAPI>;
privateKey?: HexString | Uint8Array;
fillGasPrice?: boolean;
fillGasLimit?: boolean;
}): Promise<ReturnType> {
const tx = { ...options.transaction };
return tx as unknown as ReturnType;
}

context.transactionBuilder = transactionBuilder;

// context.transactionBuilder = defaultTransactionBuilder;

const ethereumjsTx = await prepareTransactionForSigning(
{
chainId: 1458,
nonce: 1,
gasPrice: BigInt(20000000000),
gas: BigInt(21000),
to: '0xF0109fC8DF283027b6285cc889F5aA624EaC1F55',
from: '0x2c7536E3605D9C16a7a3D7b1898e529396a65c23',
value: '1000000000',
input: '',
networkId: 999,
hardfork: Hardfork.Chainstart,
chain: 'mainnet',
},
context,
);
expect(ethereumjsTx.common.hardfork()).toBe(Hardfork.Chainstart);
expect(ethereumjsTx.common.networkId().toString()).toBe('999');
});

describe('should return an web3-utils/tx instance with expected properties', () => {
it.each(validTransactions)(
'mockBlock: %s\nexpectedTransaction: %s\nexpectedPrivateKey: %s\nexpectedAddress: %s\nexpectedRlpEncodedTransaction: %s\nexpectedTransactionHash: %s\nexpectedMessageToSign: %s\nexpectedV: %s\nexpectedR: %s\nexpectedS: %s',
Expand Down
Loading

0 comments on commit 6c075db

Please sign in to comment.