Skip to content

Update EIP-1559 compatibility during network lookup #1236

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.

Sign up for GitHub

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

Merged
Gudahtt merged 2 commits into from
Apr 21, 2023
Merged

Update EIP-1559 compatibility during network lookup #1236

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
eded0d5
Update EIP-1559 compatibility during network lookup
Gudahtt Apr 20, 2023
4e1f13c
Update ascii diagram
Gudahtt Apr 21, 2023
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
11 changes: 8 additions & 3 deletions packages/network-controller/src/NetworkController.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -302,7 +302,6 @@ export class NetworkController extends BaseControllerV2<
default:
throw new Error(`Unrecognized network type: '${type}'`);
}
this.getEIP1559Compatibility();
}

getProviderAndBlockTracker(): {
Expand Down Expand Up @@ -421,7 +420,10 @@ export class NetworkController extends BaseControllerV2<
}

/**
* Refreshes the current network code.
* Performs side effects after switching to a network. If the network is
* available, updates the network state with the network ID of the network and
* stores whether the network supports EIP-1559; otherwise clears said
* information about the network that may have been previously stored.
*/
async lookupNetwork() {
if (!this.#ethQuery) {
Expand All @@ -431,7 +433,10 @@ export class NetworkController extends BaseControllerV2<

try {
try {
const networkId = await this.#getNetworkId();
const [networkId] = await Promise.all([
this.#getNetworkId(),
this.getEIP1559Compatibility(),
]);
if (this.state.networkId === networkId) {
return;
}
Expand Down
139 changes: 135 additions & 4 deletions packages/network-controller/tests/NetworkController.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,8 +107,8 @@ const POST_1559_BLOCK = {
// initializeProvider refreshNetwork
// │ │ └────────────────────────────────────────────┬──────────────────────────────────────────────┘ │
// │ │ configureProvider │
// │ │ ┌─────────────────────────┘ │ └─────────────────────────┐ │
// │ │ setupInfuraProvider setupStandardProvider getEIP1559Compatibility │
// │ │ ┌─────────────────────────┘ │ |
// │ │ setupInfuraProvider setupStandardProvider │
// │ │ └─────────────┬─────────────┘ │
// │ │ updateProvider │
// │ └───────────────┬───────────────┘ └───────────────────────────────┐ │
Expand Down Expand Up @@ -1295,6 +1295,137 @@ describe('NetworkController', () => {
});
});

describe('assuming that the network details of the current network are different from the network in state', () => {
it('updates the network in state to match', async () => {
const messenger = buildMessenger();
await withController(
{
messenger,
state: { networkDetails: { isEIP1559Compatible: false } },
},
async ({ controller }) => {
await setFakeProvider(controller, {
stubs: [
{
request: {
method: 'eth_getBlockByNumber',
params: ['latest', false],
},
response: {
result: {
baseFeePerGas: '0x1',
},
},
},
],
});

await controller.lookupNetwork();

expect(controller.state.networkDetails).toStrictEqual({
isEIP1559Compatible: true,
});
},
);
});
});

describe('if the network details of the current network are the same as that in state', () => {
it('does not change network in state', async () => {
const messenger = buildMessenger();
await withController(
{
messenger,
state: { networkDetails: { isEIP1559Compatible: true } },
},
async ({ controller }) => {
await setFakeProvider(controller, {
stubs: [
{
request: {
method: 'eth_getBlockByNumber',
params: ['latest', false],
},
response: {
result: {
baseFeePerGas: '0x1',
},
},
},
],
});
const promiseForNetworkDetailChanges = waitForStateChanges(
messenger,
{
propertyPath: ['networkDetails'],
},
);

await controller.lookupNetwork();

await expect(promiseForNetworkDetailChanges).toNeverResolve();
},
);
});
});

describe('if an RPC error is encountered while retrieving the network details of the current network', () => {
it('updates the network in state to "unavailable"', async () => {
const messenger = buildMessenger();
await withController(
{ messenger, state: { networkId: '1' } },
async ({ controller }) => {
await setFakeProvider(controller, {
stubs: [
{
request: {
method: 'eth_getBlockByNumber',
params: ['latest', false],
},
error: ethErrors.rpc.limitExceeded('some error'),
},
],
stubGetEIP1559CompatibilityWhileSetting: true,
});

await controller.lookupNetwork();

expect(controller.state.networkStatus).toBe(
NetworkStatus.Unavailable,
);
},
);
});
});

describe('if an internal error is encountered while retrieving the network details of the current network', () => {
it('updates the network in state to "unknown"', async () => {
const messenger = buildMessenger();
await withController(
{ messenger, state: { networkId: '1' } },
async ({ controller }) => {
await setFakeProvider(controller, {
stubs: [
{
request: {
method: 'eth_getBlockByNumber',
params: ['latest', false],
},
error: ethErrors.rpc.internal('some error'),
},
],
});

await controller.lookupNetwork();

expect(controller.state.networkStatus).toBe(
NetworkStatus.Unknown,
);
},
);
});
});

describe('if lookupNetwork is called multiple times in quick succession', () => {
it('waits until each call finishes before resolving the next', async () => {
const messenger = buildMessenger();
Expand Down Expand Up @@ -4975,7 +5106,7 @@ describe('NetworkController', () => {

await waitForStateChanges(messenger, {
propertyPath: ['networkId'],
count: 1,
count: 2,
produceStateChanges: () => {
controller.rollbackToPreviousProvider();
},
Expand Down Expand Up @@ -5445,7 +5576,7 @@ describe('NetworkController', () => {

await waitForStateChanges(messenger, {
propertyPath: ['networkId'],
count: 1,
count: 2,
Copy link
Copy Markdown
Member Author

@Gudahtt Gudahtt Apr 21, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

It seems that the network ID we're checking for here was always the second state change. But the second state change was made synchronously after the first one, so the test still passed.

produceStateChanges: () => {
controller.rollbackToPreviousProvider();
},
Expand Down