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

Nikos/5711/transaction with local wallet index should support to as a wallet index #5731

Merged
Show file tree
Hide file tree
Changes from 11 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
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -945,3 +945,9 @@ should use 4.0.1-alpha.0 for testing.
- These types were moved from `web3-eth-accounts` to `web3-types` package: Cipher, CipherOptions, ScryptParams, PBKDF2SHA256Params, KeyStore (#5581 )

## [Unreleased]

### Fixed

#### web3-eth

- Enable transaction with local wallet index in the `to` field (#5731)
nikoulai marked this conversation as resolved.
Show resolved Hide resolved
1 change: 1 addition & 0 deletions packages/web3-errors/src/error_codes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ export const ERR_TX_GAS_MISMATCH = 434;

export const ERR_TX_CHAIN_MISMATCH = 435;
export const ERR_TX_HARDFORK_MISMATCH = 436;
export const ERR_TX_INVALID_RECEIVER = 437;

// Connection error codes
export const ERR_CONN = 500;
Expand Down
7 changes: 7 additions & 0 deletions packages/web3-errors/src/errors/transaction_errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ import {
ERR_TX_INVALID_NONCE_OR_CHAIN_ID,
ERR_TX_INVALID_OBJECT,
ERR_TX_INVALID_SENDER,
ERR_TX_INVALID_RECEIVER,
ERR_TX_LOCAL_WALLET_NOT_AVAILABLE,
ERR_TX_MISSING_CHAIN_INFO,
ERR_TX_MISSING_CUSTOM_CHAIN,
Expand Down Expand Up @@ -168,7 +169,13 @@ export class InvalidTransactionWithSender extends InvalidValueError {
super(value, 'invalid transaction with sender');
}
}
export class InvalidTransactionWithReceiver extends InvalidValueError {
public code = ERR_TX_INVALID_RECEIVER;

public constructor(value: unknown) {
super(value, 'invalid transaction with receiver');
}
}
export class InvalidTransactionCall extends InvalidValueError {
public code = ERR_TX_INVALID_CALL;

Expand Down
4 changes: 4 additions & 0 deletions packages/web3-eth/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -68,3 +68,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Updated Web3.js dependencies (#5664)

## [Unreleased]

### Fixed

- Enable transaction with local wallet index in the `to` field (#5731)
3 changes: 2 additions & 1 deletion packages/web3-eth/src/rpc_method_wrappers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ import {
SendTransactionOptions,
} from './types';
// eslint-disable-next-line import/no-cycle
import { getTransactionFromAttr } from './utils/transaction_builder';
import { getTransactionFromAttr, getTransactionToAttr } from './utils/transaction_builder';
import { formatTransaction } from './utils/format_transaction';
// eslint-disable-next-line import/no-cycle
import { getTransactionGasPricing } from './utils/get_transaction_gas_pricing';
Expand Down Expand Up @@ -1063,6 +1063,7 @@ export function sendTransaction<
{
...transaction,
from: getTransactionFromAttr(web3Context, transaction),
to: getTransactionToAttr(web3Context, transaction),
},
ETH_DATA_FORMAT,
);
Expand Down
34 changes: 24 additions & 10 deletions packages/web3-eth/src/utils/transaction_builder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,13 +33,15 @@ import {
TransactionWithLocalWalletIndex,
Common,
Web3NetAPI,
Numbers,
} from 'web3-types';
import { Web3Context } from 'web3-core';
import { privateKeyToAddress } from 'web3-eth-accounts';
import { getId } from 'web3-net';
import { isNullish, isNumber } from 'web3-validator';
import {
InvalidTransactionWithSender,
InvalidTransactionWithReceiver,
LocalWalletNotAvailableError,
TransactionDataAndInputError,
UnableToPopulateNonceError,
Expand All @@ -53,19 +55,22 @@ import { getTransactionGasPricing } from './get_transaction_gas_pricing';
import { transactionSchema } from '../schemas';
import { InternalTransaction } from '../types';

export const getTransactionFromAttr = (
const isFromAttr = (attr: 'from' | 'to'): attr is 'from' => attr === 'from';
nikoulai marked this conversation as resolved.
Show resolved Hide resolved

const getTransactionFromOrToAttr = (
attr: 'from' | 'to',
web3Context: Web3Context<EthExecutionAPI>,
transaction?: Transaction | TransactionWithLocalWalletIndex,
privateKey?: HexString | Buffer,
) => {
if (transaction?.from !== undefined) {
if (typeof transaction.from === 'string' && isAddress(transaction.from)) {
return transaction.from;
): Address | undefined => {
if (transaction?.[attr] !== undefined) {
if (typeof transaction[attr] === 'string' && isAddress(transaction[attr] as string)) {
return transaction[attr] as Address;
}
if (isNumber(transaction.from)) {
if (isNumber(transaction[attr] as Numbers)) {
if (web3Context.wallet) {
const account = web3Context.wallet.get(
format({ eth: 'uint' }, transaction.from, NUMBER_DATA_FORMAT),
format({ eth: 'uint' }, transaction[attr] as Numbers, NUMBER_DATA_FORMAT),
);

if (!isNullish(account)) {
Expand All @@ -76,15 +81,24 @@ export const getTransactionFromAttr = (
}
throw new LocalWalletNotAvailableError();
} else {
throw new InvalidTransactionWithSender(transaction.from);
throw isFromAttr(attr)
? new InvalidTransactionWithSender(transaction.from)
: // eslint-disable-next-line @typescript-eslint/no-unsafe-call
new InvalidTransactionWithReceiver(transaction.to);
}
}
if (!isNullish(privateKey)) return privateKeyToAddress(privateKey);
if (!isNullish(web3Context.defaultAccount)) return web3Context.defaultAccount;
if (isFromAttr(attr)) {
if (!isNullish(privateKey)) return privateKeyToAddress(privateKey);
if (!isNullish(web3Context.defaultAccount)) return web3Context.defaultAccount;
}

return undefined;
};

export const getTransactionFromAttr = getTransactionFromOrToAttr.bind(undefined, 'from');

export const getTransactionToAttr = getTransactionFromOrToAttr.bind(undefined, 'to');

nikoulai marked this conversation as resolved.
Show resolved Hide resolved
export const getTransactionNonce = async <ReturnFormat extends DataFormat>(
web3Context: Web3Context<EthExecutionAPI>,
address?: Address,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,69 @@ describe('Web3Eth.sendTransaction', () => {
});
});

it('should make a simple value transfer - with local wallet indexed receiver', async () => {
const web3EthWithWallet = new Web3Eth(getSystemTestProvider());
const accountProvider = createAccountProvider(web3Eth);
const wallet = new Wallet(accountProvider);

web3EthWithWallet['_accountProvider'] = accountProvider;
web3EthWithWallet['_wallet'] = wallet;

web3EthWithWallet.wallet?.add(tempAcc.privateKey);

const transaction: TransactionWithLocalWalletIndex = {
from: tempAcc.address,
to: 0,
gas: 21000,
value: BigInt(1),
};
const response = await web3EthWithWallet.sendTransaction(transaction);
expect(response.status).toBe(BigInt(1));

const minedTransactionData = await web3EthWithWallet.getTransaction(
response.transactionHash,
);

expect(minedTransactionData).toMatchObject({
from: tempAcc.address,
to: wallet.get(0)?.address.toLocaleLowerCase(),
nikoulai marked this conversation as resolved.
Show resolved Hide resolved
value: BigInt(1),
});
});

it('should make a simple value transfer - with local wallet indexed sender and receiver', async () => {
const web3EthWithWallet = new Web3Eth(getSystemTestProvider());
const accountProvider = createAccountProvider(web3Eth);
const wallet = new Wallet(accountProvider);

web3EthWithWallet['_accountProvider'] = accountProvider;
web3EthWithWallet['_wallet'] = wallet;

const tempAcc2 = await createTempAccount();

web3EthWithWallet.wallet?.add(tempAcc.privateKey);

web3EthWithWallet.wallet?.add(tempAcc2.privateKey);

const transaction: TransactionWithLocalWalletIndex = {
from: 0,
to: 1,
gas: 21000,
value: BigInt(1),
};
const response = await web3EthWithWallet.sendTransaction(transaction);
expect(response.status).toBe(BigInt(1));

const minedTransactionData = await web3EthWithWallet.getTransaction(
response.transactionHash,
);

expect(minedTransactionData).toMatchObject({
from: tempAcc.address,
to: wallet.get(1)?.address.toLocaleLowerCase(),
nikoulai marked this conversation as resolved.
Show resolved Hide resolved
value: BigInt(1),
});
});
it('should make a transaction with no value transfer', async () => {
const transaction: Transaction = {
from: tempAcc.address,
Expand Down
4 changes: 3 additions & 1 deletion packages/web3-types/src/eth_types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -367,8 +367,10 @@ export interface TransactionCall extends Transaction {
to: Address;
}

export interface TransactionWithLocalWalletIndex extends TransactionBase {
export interface TransactionWithLocalWalletIndex extends Omit<TransactionBase, 'to'> {
from?: Numbers;
// eslint-disable-next-line @typescript-eslint/ban-types
to?: Address | Numbers | null;
Copy link
Contributor

Choose a reason for hiding this comment

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

to being Address | null is already handled by the type TransactionBase, so it should just be listed as Numbers here. The problem is from could be the local wallet index while to is Address | null, so I see why you did this, but it doesn't work for the inverse where to is an index but from is Address | null

I'm not sure how else to this besides introducing the following types: TransactionWithFromLocalWalletIndex, TransactionWithToLocalWalletIndex, and TransactionWithFromAndToLocalWalletIndex, so definitely open to suggestions. I've opened #5748 to implement this (doesn't update the CHANGELOGs, so that will need to be done if you decided to merge it)

}

export interface TransactionInfo extends Transaction {
Expand Down