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

feat: create merger and factory for the Caip25Permission #5283

Merged
merged 28 commits into from
Feb 25, 2025
Merged
Show file tree
Hide file tree
Changes from 21 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
d240951
feat: add merger function to caip25 specification builder
ffmcgee725 Feb 4, 2025
6fc8f08
refactor: code clean up
ffmcgee725 Feb 4, 2025
a057674
refactor: minor change
ffmcgee725 Feb 4, 2025
19d8725
test: unit tests for merger
ffmcgee725 Feb 5, 2025
de396ba
feat: add factory method to caip25 specification builder
ffmcgee725 Feb 5, 2025
058a08c
address code review
ffmcgee725 Feb 5, 2025
29965df
test: fix nested describe block
ffmcgee725 Feb 5, 2025
beda24f
test: fix builds the expected permission specification after factory …
ffmcgee725 Feb 5, 2025
99939da
test: add test for factory method
ffmcgee725 Feb 6, 2025
145cc29
refactor: code review changes
ffmcgee725 Feb 11, 2025
c2d3d9d
Merge branch 'main' into jc/merger-caip25-permission
ffmcgee725 Feb 11, 2025
40bec46
lint
ffmcgee725 Feb 11, 2025
1f9e0de
lint:eslint
ffmcgee725 Feb 11, 2025
dbaeb6e
code review changes
ffmcgee725 Feb 12, 2025
c32d5c7
ci
ffmcgee725 Feb 12, 2025
8d56917
fix
ffmcgee725 Feb 12, 2025
7fa8016
lint:eslint
ffmcgee725 Feb 12, 2025
0bb6877
code review
ffmcgee725 Feb 12, 2025
757b88a
code review changes
ffmcgee725 Feb 22, 2025
a910389
minor fix
ffmcgee725 Feb 22, 2025
f50f226
Merge branch 'main' into jc/merger-caip25-permission
ffmcgee725 Feb 22, 2025
f8a9da7
Merge branch 'main' into jc/merger-caip25-permission
ffmcgee725 Feb 25, 2025
a4bba23
test: minor adjustments
ffmcgee725 Feb 25, 2025
b80e9d2
test: fix
ffmcgee725 Feb 25, 2025
4947852
refactor: nit
ffmcgee725 Feb 25, 2025
0f0a6cd
Merge branch 'main' into jc/merger-caip25-permission
ffmcgee725 Feb 25, 2025
a3aa8f3
lint
ffmcgee725 Feb 25, 2025
d9f3a60
Merge branch 'main' into jc/merger-caip25-permission
ffmcgee725 Feb 25, 2025
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: 0 additions & 6 deletions eslint-warning-thresholds.json
Original file line number Diff line number Diff line change
Expand Up @@ -369,12 +369,6 @@
"@typescript-eslint/no-unsafe-enum-comparison": 4,
"jsdoc/tag-lines": 4
},
"packages/multichain/src/scope/transform.ts": {
"jsdoc/tag-lines": 3
},
"packages/multichain/src/scope/types.ts": {
"jsdoc/tag-lines": 1
},
"packages/multichain/src/scope/validation.ts": {
"jsdoc/tag-lines": 2
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import {
KnownWalletNamespaceRpcMethods,
KnownWalletRpcMethods,
} from '../scope/constants';
import { mergeScopes } from '../scope/transform';
import { mergeNormalizedScopes } from '../scope/transform';
import type {
InternalScopesObject,
NonWalletKnownCaipNamespace,
Expand Down Expand Up @@ -94,7 +94,7 @@ export const getSessionScopes = (
'requiredScopes' | 'optionalScopes'
>,
) => {
return mergeScopes(
return mergeNormalizedScopes(
getNormalizedScopesObject(caip25CaveatValue.requiredScopes),
getNormalizedScopesObject(caip25CaveatValue.optionalScopes),
);
Expand Down
264 changes: 263 additions & 1 deletion packages/multichain/src/caip25Permission.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
Caip25CaveatMutators,
createCaip25Caveat,
caip25CaveatBuilder,
diffScopesForCaip25CaveatValue,
} from './caip25Permission';
import * as ScopeSupported from './scope/supported';

Expand Down Expand Up @@ -475,7 +476,7 @@ describe('caip25EndowmentBuilder', () => {
describe('caip25CaveatBuilder', () => {
const findNetworkClientIdByChainId = jest.fn();
const listAccounts = jest.fn();
const { validator } = caip25CaveatBuilder({
const { validator, merger } = caip25CaveatBuilder({
findNetworkClientIdByChainId,
listAccounts,
});
Expand Down Expand Up @@ -698,4 +699,265 @@ describe('caip25CaveatBuilder', () => {
}),
).toBeUndefined();
});

describe('permission merger', () => {
describe('incremental request an existing scope (requiredScopes), and 2 whole new scopes (optionalScopes) with accounts', () => {
it('should return merged scope with previously existing chain and accounts, plus new requested chains with new accounts', () => {
const initLeftValue: Caip25CaveatValue = {
requiredScopes: {
'eip155:1': {
accounts: ['eip155:1:0xdead'],
},
},
optionalScopes: {},
isMultichainOrigin: false,
};

const rightValue: Caip25CaveatValue = {
requiredScopes: {},
optionalScopes: {
'eip155:1': {
accounts: ['eip155:1:0xdead', 'eip155:1:0xbadd'],
},
'eip155:10': {
accounts: ['eip155:10:0xbeef', 'eip155:10:0xbadd'],
},
'eip155:426161': {
accounts: [
'eip155:426161:0xdead',
'eip155:426161:0xbeef',
'eip155:426161:0xbadd',
],
},
},
isMultichainOrigin: false,
};

const expectedMergedValue: Caip25CaveatValue = {
requiredScopes: {
'eip155:1': { accounts: ['eip155:1:0xdead'] },
},
optionalScopes: {
'eip155:1': { accounts: ['eip155:1:0xdead', 'eip155:1:0xbadd'] },
'eip155:10': {
accounts: ['eip155:10:0xbeef', 'eip155:10:0xbadd'],
},
'eip155:426161': {
accounts: [
'eip155:426161:0xdead',
'eip155:426161:0xbeef',
'eip155:426161:0xbadd',
],
},
},
isMultichainOrigin: false,
};
const expectedDiff: Caip25CaveatValue = {
requiredScopes: {},
optionalScopes: {
'eip155:1': { accounts: ['eip155:1:0xdead', 'eip155:1:0xbadd'] },
'eip155:10': {
accounts: ['eip155:10:0xbeef', 'eip155:10:0xbadd'],
},
'eip155:426161': {
accounts: [
'eip155:426161:0xdead',
'eip155:426161:0xbeef',
'eip155:426161:0xbadd',
],
},
},
isMultichainOrigin: false,
};
const [newValue, diff] = merger(initLeftValue, rightValue);

expect(newValue).toStrictEqual(
expect.objectContaining(expectedMergedValue),
);
expect(diff).toStrictEqual(expect.objectContaining(expectedDiff));
});
});
});
});

describe('diffScopesForCaip25CaveatValue', () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, it looks like we are still just testing this function with optionalScopes only, so the fact that it accepts requiredScopes is not being tested. Maybe we can copy these four describes and then change them so that we are setting requiredScopes?

Copy link
Member Author

Choose a reason for hiding this comment

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

Addressed in a4bba23 and b80e9d2

describe('incremental request existing scope with a new account', () => {
it('should return scope with existing chain and new requested account', () => {
const leftValue: Caip25CaveatValue = {
optionalScopes: {
'eip155:1': {
accounts: ['eip155:1:0xdead'],
},
},
requiredScopes: {},
isMultichainOrigin: false,
};

const mergedValue: Caip25CaveatValue = {
optionalScopes: {
'eip155:1': {
accounts: ['eip155:1:0xdead', 'eip155:1:0xbeef'],
},
},
requiredScopes: {},
isMultichainOrigin: false,
};

const expectedDiff: Caip25CaveatValue = {
optionalScopes: {
'eip155:1': {
accounts: ['eip155:1:0xbeef'],
},
},
isMultichainOrigin: false,
requiredScopes: {},
};

const diff = diffScopesForCaip25CaveatValue(
leftValue,
mergedValue,
'optionalScopes',
);

expect(diff).toStrictEqual(expect.objectContaining(expectedDiff));
});
});

describe('incremental request a whole new scope without accounts', () => {
it('should return scope with new requested chain and no accounts', () => {
const leftValue: Caip25CaveatValue = {
optionalScopes: {
'eip155:1': {
accounts: ['eip155:1:0xdead'],
},
},
requiredScopes: {},
isMultichainOrigin: false,
};

const mergedValue: Caip25CaveatValue = {
optionalScopes: {
'eip155:1': {
accounts: ['eip155:1:0xdead'],
},
'eip155:10': {
accounts: [],
},
},
requiredScopes: {},
isMultichainOrigin: false,
};

const expectedDiff: Caip25CaveatValue = {
optionalScopes: {
'eip155:10': {
accounts: [],
},
},
isMultichainOrigin: false,
requiredScopes: {},
};

const diff = diffScopesForCaip25CaveatValue(
leftValue,
mergedValue,
'optionalScopes',
);

expect(diff).toStrictEqual(expect.objectContaining(expectedDiff));
});
});

describe('incremental request a whole new scope with accounts', () => {
it('should return scope with new requested chain and new account', () => {
const leftValue: Caip25CaveatValue = {
optionalScopes: {
'eip155:1': {
accounts: ['eip155:1:0xdead'],
},
},
requiredScopes: {},
isMultichainOrigin: false,
};

const mergedValue: Caip25CaveatValue = {
optionalScopes: {
'eip155:1': {
accounts: ['eip155:1:0xdead'],
},
'eip155:10': {
accounts: ['eip155:10:0xbeef'],
},
},
requiredScopes: {},
isMultichainOrigin: false,
};

const expectedDiff: Caip25CaveatValue = {
optionalScopes: {
'eip155:10': {
accounts: ['eip155:10:0xbeef'],
},
},
isMultichainOrigin: false,
requiredScopes: {},
};

const diff = diffScopesForCaip25CaveatValue(
leftValue,
mergedValue,
'optionalScopes',
);

expect(diff).toStrictEqual(expect.objectContaining(expectedDiff));
});
});

describe('incremental request an existing scope with new accounts, and whole new scope with accounts', () => {
it('should return scope with previously existing chain and accounts, plus new requested chain with new accounts', () => {
const leftValue: Caip25CaveatValue = {
optionalScopes: {
'eip155:1': {
accounts: ['eip155:1:0xdead'],
},
},
requiredScopes: {},
isMultichainOrigin: false,
};

const mergedValue: Caip25CaveatValue = {
optionalScopes: {
'eip155:1': {
accounts: ['eip155:1:0xdead', 'eip155:1:0xbeef'],
},
'eip155:10': {
accounts: ['eip155:10:0xdead', 'eip155:10:0xbeef'],
},
},
requiredScopes: {},
isMultichainOrigin: false,
};

const expectedDiff: Caip25CaveatValue = {
optionalScopes: {
'eip155:1': {
accounts: ['eip155:1:0xbeef'],
},
'eip155:10': {
accounts: ['eip155:10:0xdead', 'eip155:10:0xbeef'],
},
},
isMultichainOrigin: false,
requiredScopes: {},
};

const diff = diffScopesForCaip25CaveatValue(
leftValue,
mergedValue,
'optionalScopes',
);

expect(diff).toStrictEqual(expect.objectContaining(expectedDiff));
});
});
});
Loading
Loading