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

5704 memory leak in web3.eth.contract #5792

Merged
merged 12 commits into from
Jan 31, 2023
Merged
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
10 changes: 10 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -1138,3 +1138,13 @@ should use 4.0.1-alpha.0 for testing.
## [Unreleased]

- Added rpc exception codes following eip-1474 as an experimental feature (if `useRpcCallSpecification` at `enableExperimentalFeatures` is `true`) (#5525)

#### web3

##### Removed

- Private static `_contracts:Contract[]` and static `setProvider` function was removed (#5792)

##### Added

- `registeredSubscriptions` was added by default in web3 constructor (#5792)
2 changes: 1 addition & 1 deletion packages/web3-eth/src/web3_eth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ type RegisteredSubscription = {
syncing: typeof SyncingSubscription;
};

const registeredSubscriptions = {
export const registeredSubscriptions = {
logs: LogsSubscription,
newPendingTransactions: NewPendingTransactionsSubscription,
newHeads: NewHeadsSubscription,
Expand Down
8 changes: 8 additions & 0 deletions packages/web3/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -66,3 +66,11 @@ web3.currentProvider.disconnect();
- `build` entry from `package.json` (#5755)

## [Unreleased]

### Removed

- Private static `_contracts:Contract[]` and static `setProvider` function was removed (#5792)

### Added

- `registeredSubscriptions` was added by default in web3 constructor (#5792)
8 changes: 3 additions & 5 deletions packages/web3/src/types.ts
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 { EthExecutionAPI, SupportedProviders, Address, Bytes, Transaction } from 'web3-types';
import { Address, Bytes, Transaction } from 'web3-types';
import Eth from 'web3-eth';
import {
ContractAbi,
Expand All @@ -42,10 +42,8 @@ import { ENS } from 'web3-eth-ens';
import Net from 'web3-net';
import { Iban } from 'web3-eth-iban';

export type Web3ContractConstructor<Abi extends ContractAbi> = Omit<typeof Contract, 'new'> & {
new (jsonInterface: Abi, address?: Address, options?: ContractInitOptions): Contract<Abi>;
setProvider: (provider: SupportedProviders<EthExecutionAPI>) => void;
};
export type Web3ContractConstructor<Abi extends ContractAbi> = Omit<typeof Contract, 'new'> &
(new (jsonInterface: Abi, address?: Address, options?: ContractInitOptions) => Contract<Abi>);

/**
* The Ethereum interface for main web3 object. It provides extra methods in addition to `web3-eth` interface.
Expand Down
17 changes: 2 additions & 15 deletions packages/web3/src/web3.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ along with web3.js. If not, see <http://www.gnu.org/licenses/>.
*/
// eslint-disable-next-line max-classes-per-file
import { Web3Context } from 'web3-core';
import Web3Eth from 'web3-eth';
import Web3Eth, { registeredSubscriptions } from 'web3-eth';
import { ContractAbi } from 'web3-eth-abi';
import Contract, { ContractInitOptions } from 'web3-eth-contract';
import { ENS, registryAddresses } from 'web3-eth-ens';
Expand Down Expand Up @@ -47,7 +47,7 @@ export class Web3 extends Web3Context<EthExecutionAPI> {
public eth: Web3EthInterface;

public constructor(provider?: SupportedProviders<EthExecutionAPI> | string) {
super({ provider });
super({ provider, registeredSubscriptions });

if (isNullish(provider) || (typeof provider === 'string' && provider.trim() === '')) {
console.warn(
Expand All @@ -68,9 +68,6 @@ export class Web3 extends Web3Context<EthExecutionAPI> {
const self = this;

class ContractBuilder<Abi extends ContractAbi> extends Contract<Abi> {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
private static readonly _contracts: Contract<any>[] = [];

public constructor(jsonInterface: Abi);
public constructor(jsonInterface: Abi, address: Address);
public constructor(jsonInterface: Abi, options: ContractInitOptions);
Expand All @@ -91,16 +88,6 @@ export class Web3 extends Web3Context<EthExecutionAPI> {
} else {
super(jsonInterface, self.getContextObject());
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Also update change log for this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

// eslint-disable-next-line @typescript-eslint/no-explicit-any
ContractBuilder._contracts.push(this as Contract<any>);
}

public static setProvider(_provider: SupportedProviders<EthExecutionAPI>): boolean {
for (const contract of ContractBuilder._contracts) {
contract.provider = _provider;
}
return true;
}
}

Expand Down
80 changes: 80 additions & 0 deletions packages/web3/test/integration/web3.setProvider.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
/*
This file is part of web3.js.

web3.js is free software: you can redistribute it and/or modify
it under the terms of the GNU Lesser General Public License as published by
the Free Software Foundation, either version 3 of the License, or
(at your option) any later version.

web3.js is distributed in the hope that it will be useful,
but WITHOUT ANY WARRANTY; without even the implied warranty of
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
GNU Lesser General Public License for more details.

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 {
getSystemTestProvider,
waitForOpenConnection,
closeOpenConnection,
isWs,
isHttp,
describeIf,
} from '../shared_fixtures/system_tests_utils';
import Web3 from '../../src/index';

describeIf(isWs || isHttp)('web3.contract.setProvider', () => {
let clientUrl: string;
let secontUrl: string;
let web3: Web3;

beforeAll(async () => {
clientUrl = getSystemTestProvider();
secontUrl = clientUrl.startsWith('http')
? clientUrl.replace('http', 'ws')
: clientUrl.replace('ws', 'http');
web3 = new Web3(clientUrl);

await waitForOpenConnection(web3);
});

afterAll(async () => {
await closeOpenConnection(web3);
});

test('create few contracts and check providers', () => {
const c1 = new web3.eth.Contract([]);
const c2 = new web3.eth.Contract([]);

expect(c1.provider).toBe(web3.provider);
expect(c2.provider).toBe(web3.provider);
});

test('create few contracts and check providers. set different provider', () => {
const c1 = new web3.eth.Contract([]);
const c2 = new web3.eth.Contract([]);

expect(c1.provider).toBe(web3.provider);
expect(c2.provider).toBe(web3.provider);

web3.setProvider(secontUrl);

expect(c1.provider).toBe(web3.provider);
expect(c2.provider).toBe(web3.provider);
});

test('create few contracts, set different provider to contract and check other contract', () => {
const c1 = new web3.eth.Contract([]);
const c2 = new web3.eth.Contract([]);

expect(c1.provider).toBe(web3.provider);
expect(c2.provider).toBe(web3.provider);

c1.setProvider(secontUrl);

expect(c1.provider).toBe(web3.provider);
expect(c2.provider).toBe(web3.provider);
});
});
3 changes: 2 additions & 1 deletion scripts/system_tests_utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,7 @@ import Web3 from 'web3';
// eslint-disable-next-line import/no-extraneous-dependencies
import { NonPayableMethodObject } from 'web3-eth-contract';
// eslint-disable-next-line import/no-extraneous-dependencies
import HttpProvider from 'web3-providers-http';
import accountsString from './accounts.json';

/**
Expand Down Expand Up @@ -125,7 +126,7 @@ export const waitForOpenConnection = async (
});

export const closeOpenConnection = async (web3Context: Web3Context) => {
if (!isSocket) {
if (!isSocket || web3Context?.provider instanceof HttpProvider) {
return;
}

Expand Down