Skip to content
This repository was archived by the owner on Mar 5, 2025. It is now read-only.

Commit 6d04bb1

Browse files
nazarhussainnazarhussain
authored
Fix bugs for the wallet and PromiEvent (#5175)
* ✨ Add integer index support to wallet * ✨ Add event chaining suppor to PromiEvent * 🎨 Fix the build errors * ✅ Add one extra test case for private key * 🚨 Fix linter errors
1 parent a566423 commit 6d04bb1

File tree

17 files changed

+360
-250
lines changed

17 files changed

+360
-250
lines changed

‎packages/web3-common/src/web3_base_wallet.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,16 +50,17 @@ export interface Web3AccountProvider<T> {
5050
decrypt: (keystore: string, password: string, options?: Record<string, unknown>) => Promise<T>;
5151
}
5252

53-
export abstract class Web3BaseWallet<T extends Web3BaseWalletAccount> {
53+
export abstract class Web3BaseWallet<T extends Web3BaseWalletAccount> extends Array<T> {
5454
protected readonly _accountProvider: Web3AccountProvider<T>;
5555

5656
public constructor(accountProvider: Web3AccountProvider<T>) {
57+
super();
5758
this._accountProvider = accountProvider;
5859
}
5960

6061
public abstract create(numberOfAccounts: number): this;
6162
public abstract add(account: T | string): boolean;
62-
public abstract get(addressOrIndex: string | number): T;
63+
public abstract get(addressOrIndex: string | number): T | undefined;
6364
public abstract remove(addressOrIndex: string | number): boolean;
6465
public abstract clear(): this;
6566
public abstract encrypt(

‎packages/web3-common/src/web3_promi_event.ts‎

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,12 @@ You should have received a copy of the GNU Lesser General Public License
1515
along with web3.js. If not, see <http://www.gnu.org/licenses/>.
1616
*/
1717

18-
import { Web3EventEmitter, Web3EventMap } from './web3_event_emitter';
18+
import {
19+
Web3EventCallback,
20+
Web3EventEmitter,
21+
Web3EventKey,
22+
Web3EventMap,
23+
} from './web3_event_emitter';
1924

2025
export type PromiseExecutor<T> = (
2126
resolve: (data: T) => void,
@@ -52,4 +57,22 @@ export class Web3PromiEvent<ResolveType, EventMap extends Web3EventMap>
5257
public async finally(onfinally?: (() => void) | undefined): Promise<ResolveType> {
5358
return this._promise.finally(onfinally);
5459
}
60+
61+
public on<K extends Web3EventKey<EventMap>>(
62+
eventName: K,
63+
fn: Web3EventCallback<EventMap[K]>,
64+
): this {
65+
super.on(eventName, fn);
66+
67+
return this;
68+
}
69+
70+
public once<K extends Web3EventKey<EventMap>>(
71+
eventName: K,
72+
fn: Web3EventCallback<EventMap[K]>,
73+
): this {
74+
super.once(eventName, fn);
75+
76+
return this;
77+
}
5578
}

‎packages/web3-common/test/unit/web3_promi_event.test.ts‎

Lines changed: 45 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -41,15 +41,16 @@ describe('Web3PromiEvent', () => {
4141
});
4242

4343
p.on('data', data => {
44+
// eslint-disable-next-line jest/no-conditional-expect
4445
expect(data).toBe('resolved value');
4546
done(undefined);
46-
});
47+
})
48+
.then(data => {
49+
p.emit('data', data);
50+
})
51+
.catch(e => done(e));
4752

48-
p.then(data => {
49-
p.emit('data', data);
50-
}).catch(e => {
51-
throw e;
52-
});
53+
expect.assertions(1);
5354
});
5455
});
5556

@@ -69,10 +70,47 @@ describe('Web3PromiEvent', () => {
6970

7071
const p = func();
7172

72-
p.on('data', data => {
73+
// eslint-disable-next-line no-void
74+
void p.on('data', data => {
7375
expect(data).toBe('emitted data');
7476
done(undefined);
7577
});
7678
});
7779
});
80+
81+
it('should return the promi-event object from "on" handler', async () => {
82+
const p = new Web3PromiEvent<string, { event1: string; event2: number }>(resolve => {
83+
resolve('resolved value');
84+
})
85+
.on('event1', data => {
86+
expect(data).toBe('string value');
87+
})
88+
.on('event2', data => {
89+
expect(data).toBe(3);
90+
});
91+
92+
p.emit('event1', 'string value');
93+
p.emit('event2', 3);
94+
95+
await expect(p).resolves.toBe('resolved value');
96+
expect.assertions(3);
97+
});
98+
99+
it('should return the promi-event object from "once" handler', async () => {
100+
const p = new Web3PromiEvent<string, { event1: string; event2: number }>(resolve => {
101+
resolve('resolved value');
102+
})
103+
.once('event1', data => {
104+
expect(data).toBe('string value');
105+
})
106+
.once('event2', data => {
107+
expect(data).toBe(3);
108+
});
109+
110+
p.emit('event1', 'string value');
111+
p.emit('event2', 3);
112+
113+
await expect(p).resolves.toBe('resolved value');
114+
expect.assertions(3);
115+
});
78116
});

‎packages/web3-eth-accounts/src/wallet.ts‎

Lines changed: 32 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -15,27 +15,17 @@ You should have received a copy of the GNU Lesser General Public License
1515
along with web3.js. If not, see <http://www.gnu.org/licenses/>.
1616
*/
1717

18-
import {
19-
Web3BaseWallet,
20-
Web3BaseWalletAccount,
21-
Web3AccountProvider,
22-
Web3EncryptedWallet,
23-
} from 'web3-common';
18+
import { Web3BaseWallet, Web3BaseWalletAccount, Web3EncryptedWallet } from 'web3-common';
2419
import { isNullish } from 'web3-validator';
2520

2621
type BrowserError = { code: number; name: string };
2722

2823
export class Wallet<
2924
T extends Web3BaseWalletAccount = Web3BaseWalletAccount,
3025
> extends Web3BaseWallet<T> {
31-
private readonly _accounts: { [key: string]: T };
26+
private readonly _addressMap = new Map<string, number>();
3227
private readonly _defaultKeyName = 'web3js_wallet';
3328

34-
public constructor(accountProvider: Web3AccountProvider<T>) {
35-
super(accountProvider);
36-
this._accounts = {};
37-
}
38-
3929
public static getStorage(): Storage | undefined {
4030
let storage: Storage | undefined;
4131

@@ -65,10 +55,6 @@ export class Wallet<
6555
}
6656
}
6757

68-
public get length() {
69-
return Object.keys(this._accounts).length;
70-
}
71-
7258
public create(numberOfAccounts: number) {
7359
for (let i = 0; i < numberOfAccounts; i += 1) {
7460
this.add(this._accountProvider.create());
@@ -82,45 +68,59 @@ export class Wallet<
8268
return this.add(this._accountProvider.privateKeyToAccount(account));
8369
}
8470

85-
this._accounts[account.address.toLowerCase()] = account;
71+
const index = this.length;
72+
this._addressMap.set(account.address.toLowerCase(), index);
73+
74+
this[index] = account;
8675

8776
return true;
8877
}
8978

90-
public get(addressOrIndex: string | number): T {
79+
public get(addressOrIndex: string | number): T | undefined {
9180
if (typeof addressOrIndex === 'string') {
92-
return this._accounts[addressOrIndex];
81+
const index = this._addressMap.get(addressOrIndex);
82+
83+
if (!isNullish(index)) {
84+
return this[index];
85+
}
86+
87+
return undefined;
9388
}
9489

95-
return Object.values(this._accounts)[addressOrIndex];
90+
return this[addressOrIndex];
9691
}
9792

9893
public remove(addressOrIndex: string | number): boolean {
99-
const result =
100-
typeof addressOrIndex === 'string'
101-
? { address: addressOrIndex }
102-
: Object.values(this._accounts)[addressOrIndex];
94+
if (typeof addressOrIndex === 'string') {
95+
const index = this._addressMap.get(addressOrIndex.toLowerCase());
96+
if (isNullish(index)) {
97+
return false;
98+
}
99+
this._addressMap.delete(addressOrIndex.toLowerCase());
100+
this.splice(index, 1);
101+
102+
return true;
103+
}
103104

104-
if (result && this._accounts[result.address]) {
105-
delete this._accounts[result.address];
105+
if (this[addressOrIndex]) {
106+
this.splice(addressOrIndex, 1);
106107
return true;
107108
}
108109

109110
return false;
110111
}
111112

112113
public clear() {
113-
for (const key of Object.keys(this._accounts)) {
114-
delete this._accounts[key];
115-
}
114+
this._addressMap.clear();
115+
116+
// Setting length clears the Array in JS.
117+
this.length = 0;
116118

117119
return this;
118120
}
119121

120122
public async encrypt(password: string, options?: Record<string, unknown> | undefined) {
121-
return Promise.all(
122-
Object.values(this._accounts).map(async account => account.encrypt(password, options)),
123-
);
123+
return Promise.all(this.map(async account => account.encrypt(password, options)));
124124
}
125125

126126
public async decrypt(

‎packages/web3-eth-accounts/test/unit/wallet.test.ts‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,14 @@ describe('Wallet', () => {
142142
expect(wallet.get('my_address')).toEqual(account);
143143
expect(wallet.get('my_Address')).toBeUndefined();
144144
});
145+
146+
it('should get account with index', () => {
147+
const account = { address: 'my_Address' } as never;
148+
149+
wallet.add(account);
150+
151+
expect(wallet[0]).toEqual(account);
152+
});
145153
});
146154

147155
describe('remove', () => {
@@ -190,6 +198,40 @@ describe('Wallet', () => {
190198
expect(result).toBeFalsy();
191199
expect(wallet).toHaveLength(1);
192200
});
201+
202+
it('should remove account with the index', () => {
203+
const account = { address: 'my_address' } as never;
204+
wallet.add(account);
205+
expect(wallet).toHaveLength(1);
206+
207+
delete wallet[0];
208+
209+
// Deleting objects dees not change the length
210+
expect(wallet).toHaveLength(1);
211+
expect(wallet[0]).toBeUndefined();
212+
});
213+
214+
it('should remove account with array methods', () => {
215+
const account = { address: 'my_address' } as never;
216+
wallet.add(account);
217+
expect(wallet).toHaveLength(1);
218+
219+
wallet.splice(0, 1);
220+
221+
expect(wallet).toHaveLength(0);
222+
expect(wallet[0]).toBeUndefined();
223+
});
224+
225+
it('should remove account added with private key with array methods', () => {
226+
const privateKey = 'private_key';
227+
wallet.add(privateKey);
228+
expect(wallet).toHaveLength(1);
229+
230+
wallet.splice(0, 1);
231+
232+
expect(wallet).toHaveLength(0);
233+
expect(wallet[0]).toBeUndefined();
234+
});
193235
});
194236

195237
describe('clear', () => {

‎packages/web3-eth-contract/src/contract.ts‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -452,9 +452,9 @@ export class Contract<Abi extends ContractAbi>
452452
*
453453
* The methods of this smart contract are available through:
454454
*
455-
* * The name: `myContract.methods.myMethod(123)`
456-
* * The name with parameters: `myContract.methods['myMethod(uint256)'](123)`
457-
* * The signature `myContract.methods['0x58cf5f10'](123)`
455+
* The name: `myContract.methods.myMethod(123)`
456+
* The name with parameters: `myContract.methods['myMethod(uint256)'](123)`
457+
* The signature `myContract.methods['0x58cf5f10'](123)`
458458
*
459459
* This allows calling functions with same name but different parameters from the JavaScript contract object.
460460
*
@@ -574,6 +574,9 @@ export class Contract<Abi extends ContractAbi>
574574
* });
575575
* ```
576576
*
577+
* @param deployOptions
578+
* @param deployOptions.data
579+
* @param deployOptions.arguments
577580
* @returns - The transaction object
578581
*/
579582
public deploy(deployOptions?: {
@@ -685,6 +688,7 @@ export class Contract<Abi extends ContractAbi>
685688
*
686689
* @param eventName - The name of the event in the contract, or `allEvents` to get all events.
687690
* @param filter - The filter options used to get events.
691+
* @param returnFormat
688692
* @returns - An array with the past event `Objects`, matching the given event name and filter.
689693
*/
690694
public async getPastEvents<ReturnFormat extends DataFormat = typeof DEFAULT_RETURN_FORMAT>(

‎packages/web3-eth-contract/test/integration/contract_defaults.test.ts‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -50,11 +50,10 @@ describe('contract', () => {
5050
const receiptHandler = jest.fn();
5151

5252
// We didn't specify "from" in this call
53-
const tx = contract.deploy(deployOptions).send({ gas: '1000000' });
54-
55-
tx.on('receipt', receiptHandler);
56-
57-
await tx;
53+
await contract
54+
.deploy(deployOptions)
55+
.send({ gas: '1000000' })
56+
.on('receipt', receiptHandler);
5857

5958
// We didn't specify "from" in this call
6059
expect(receiptHandler).toHaveBeenCalledWith(

0 commit comments

Comments
 (0)