Skip to content

Olu/enable ppom on more networks - #90

Open
segun wants to merge 4 commits into
mainfrom
olu/send-ppom-on-more-networks
Open

Olu/enable ppom on more networks#90
segun wants to merge 4 commits into
mainfrom
olu/send-ppom-on-more-networks

Conversation

@segun

@segunsegun commented Nov 22, 2023

Copy link
Copy Markdown
Contributor

We should enabled PPOM on the following networks:

Ethereum Mainnet (already done)
BNB chain
Polygon
Arbitrum
Optimism
Avalanche
Linea

Fixes: https://github.com/MetaMask/MetaMask-planning/issues/1693

@segunsegun self-assigned this Nov 22, 2023
@segun
segun requested a review from a team as a code ownerNovember 22, 2023 09:55
Comment threadsrc/ppom-controller.test.ts Outdated
@jpuri

Copy link
Copy Markdown
Collaborator

@segun : Can you plz test the PR in extension to ensure that validation works on multiple chains, additionally caching data files and switching between networks should work well.

@bschorchit

Copy link
Copy Markdown

@segun would you mind also including Linea on this PR? - we have been unblocked for supporting this network too and are good to go

@segunsegun changed the title Olu/send ppom on more networksOlu/enable ppom on more networksDec 4, 2023
@segun
segunforce-pushed the olu/send-ppom-on-more-networks branch 3 times, most recently from d39226b to c399d94CompareDecember 6, 2023 11:07
callBack({ providerConfig: { chainId: '0x6' } });

expect(Object.keys(ppomController.state.chainStatus)).toHaveLength(5);
jest.useFakeTimers().setSystemTime(new Date('2023-01-04'));

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

I needed to add one more because the condition in ppomController for this check to happen is > 5. It doesn't do the check if its <= 5

@segun
segunforce-pushed the olu/send-ppom-on-more-networks branch from c399d94 to 2cf1cddCompareDecember 6, 2023 15:25
chainStatus[chainId]?.versionInfo ??
this.state.versionInfo.filter(({ chainId: id }) => id === chainId);
console.log('chainStatus 1008', chainStatus);
const hasVersionInfo = chainStatus[chainId]?.versionInfo?.length;

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

I noticed this part wasn't been called in tests because chainStatus[chainId]?.versionInfo is usually [] not undefined/null. So this check makes sure it works either it's undefined, null or []

Comment threadsrc/ppom-controller.ts Outdated
digiwand
digiwand previously approved these changes Dec 6, 2023
Ethereum Mainnet (already done)
BNB chain
Polygon
Arbitrum
Optimism
Avalanche
Test case for supported chain id
Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com>
Add linea mainnet to supported chain IDs.
Lint fixes
Fix tests
Signed-off-by: Akintayo A. Olusegun <akintayo.segun@gmail.com>
@segun
segunforce-pushed the olu/send-ppom-on-more-networks branch from 33e1943 to 4976b8cCompareDecember 7, 2023 13:52
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@segun@jpuri@bschorchit@digiwand