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.

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

Jump to bottom

feat: revoke CAIP-25 endowment if only eip155 account or scope is removed #4978

Merged
merged 12 commits into from
Nov 26, 2024
Merged

feat: revoke CAIP-25 endowment if only eip155 account or scope is removed #4978

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
4015c13
revoke if last eth account or permitted chain
jiexi Nov 25, 2024
2a430f5
Merge branch 'main' into jl/caip-multichain-revoke-in-mutators
jiexi Nov 25, 2024
f197304
stop upserting 'wallet:eip155` in setEthAccounts
jiexi Nov 25, 2024
50b4a30
update removeScope consider non-eth scopes. update removeAccounts con…
jiexi Nov 25, 2024
cf061b0
Merge remote-tracking branch 'origin/jl/caip-multichain-revoke-in-mut…
jiexi Nov 25, 2024
1e8ded3
Merge branch 'main' into jl/caip-multichain-revoke-in-mutators
jiexi Nov 25, 2024
35ce68c
update setEthAccounts jsdoc
jiexi Nov 26, 2024
402e3f8
add does not revoke if only wallet:eip155 accounts spec
jiexi Nov 26, 2024
8a7e499
Merge branch 'main' into jl/caip-multichain-revoke-in-mutators
jiexi Nov 26, 2024
0e4a1d8
Merge branch 'main' into jl/caip-multichain-revoke-in-mutators
adonesky1 Nov 26, 2024
d65e8eb
cleanup specs
jiexi Nov 26, 2024
562f847
Merge remote-tracking branch 'origin/jl/caip-multichain-revoke-in-mut…
jiexi Nov 26, 2024
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
23 changes: 0 additions & 23 deletions packages/multichain/src/adapters/caip-permission-adapter-eth-accounts.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,29 +159,6 @@ describe('CAIP-25 eth_accounts adapters', () => {
});
});

it('returns a CAIP-25 caveat value with missing "wallet:eip155" optional scope filled in, forming CAIP-10 account addresses from the accounts param', () => {
const input: Caip25CaveatValue = {
requiredScopes: {},
optionalScopes: {},
isMultichainOrigin: false,
};

const result = setEthAccounts(input, ['0x1', '0x2', '0x3']);
expect(result).toStrictEqual({
requiredScopes: {},
optionalScopes: {
'wallet:eip155': {
accounts: [
'wallet:eip155:0x1',
'wallet:eip155:0x2',
'wallet:eip155:0x3',
],
},
},
isMultichainOrigin: false,
});
});

it('does not modify the input CAIP-25 caveat value object in place', () => {
const input: Caip25CaveatValue = {
requiredScopes: {
Expand Down
12 changes: 1 addition & 11 deletions packages/multichain/src/adapters/caip-permission-adapter-eth-accounts.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,11 +116,6 @@ const setEthAccountsForScopesObject = (
* Sets the Ethereum (EIP155 namespaced) accounts for the given CAIP-25 caveat value.
* We set the same accounts for all the scopes that are EIP155 or Wallet namespaced because
* we do not provide UI/UX flows for selecting different accounts across different chains.
*
* Additionally, this function adds a `wallet:eip155` scope with empty methods, notifications, and accounts
* to ensure that the `wallet:eip155` scope is always present in the caveat value.
* This is required for Snaps currently can have account permissions without chain permissions.
* This added `wallet:eip155` scope should be removed once Snaps are able to have/use chain permissions.
* @param caip25CaveatValue - The CAIP-25 caveat value to set the Ethereum accounts for.
* @param accounts - The Ethereum accounts to set.
* @returns The updated CAIP-25 caveat value with the Ethereum accounts set.
Expand All @@ -136,12 +131,7 @@ export const setEthAccounts = (
accounts,
jiexi marked this conversation as resolved.
Show resolved Hide resolved
),
optionalScopes: setEthAccountsForScopesObject(
{
[KnownWalletScopeString.Eip155]: {
accounts: [],
},
...caip25CaveatValue.optionalScopes,
},
caip25CaveatValue.optionalScopes,
accounts,
),
};
Expand Down
154 changes: 130 additions & 24 deletions packages/multichain/src/caip25Permission.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,8 @@ describe('caip25EndowmentBuilder', () => {

describe('Caip25CaveatMutators.authorizedScopes', () => {
describe('removeScope', () => {
it('returns a version of the caveat with the given scope removed from requiredScopes if it is present', () => {
const ethereumGoerliCaveat = {
it('updates the caveat with the given scope removed from requiredScopes if it is present', () => {
const caveatValue = {
requiredScopes: {
'eip155:1': {
accounts: [],
Expand All @@ -78,7 +78,7 @@ describe('caip25EndowmentBuilder', () => {
sessionProperties: {},
isMultichainOrigin: true,
};
const result = removeScope(ethereumGoerliCaveat, 'eip155:1');
const result = removeScope(caveatValue, 'eip155:1');
expect(result).toStrictEqual({
operation: CaveatMutatorOperation.UpdateValue,
value: {
Expand All @@ -92,8 +92,8 @@ describe('caip25EndowmentBuilder', () => {
});
});

it('returns a version of the caveat with the given scope removed from optionalScopes if it is present', () => {
const ethereumGoerliCaveat = {
it('updates the caveat with the given scope removed from optionalScopes if it is present', () => {
const caveatValue = {
requiredScopes: {
'eip155:1': {
accounts: [],
Expand All @@ -107,7 +107,7 @@ describe('caip25EndowmentBuilder', () => {
sessionProperties: {},
isMultichainOrigin: true,
};
const result = removeScope(ethereumGoerliCaveat, 'eip155:5');
const result = removeScope(caveatValue, 'eip155:5');
expect(result).toStrictEqual({
operation: CaveatMutatorOperation.UpdateValue,
value: {
Expand All @@ -121,8 +121,8 @@ describe('caip25EndowmentBuilder', () => {
});
});

it('returns a version of the caveat with the given scope removed from requiredScopes and optionalScopes if it is present', () => {
const ethereumGoerliCaveat = {
it('updates the caveat with the given scope removed from requiredScopes and optionalScopes if it is present', () => {
const caveatValue = {
requiredScopes: {
'eip155:1': {
accounts: [],
Expand All @@ -139,7 +139,7 @@ describe('caip25EndowmentBuilder', () => {
sessionProperties: {},
isMultichainOrigin: true,
};
const result = removeScope(ethereumGoerliCaveat, 'eip155:5');
const result = removeScope(caveatValue, 'eip155:5');
expect(result).toStrictEqual({
operation: CaveatMutatorOperation.UpdateValue,
value: {
Expand All @@ -153,8 +153,51 @@ describe('caip25EndowmentBuilder', () => {
});
});

it('returns the caveat unchanged when the given scope is not found in either requiredScopes or optionalScopes', () => {
const ethereumGoerliCaveat = {
it('revokes the permission if the only non wallet scope is removed', () => {
const caveatValue = {
requiredScopes: {},
optionalScopes: {
'eip155:5': {
accounts: [],
},
'wallet:eip155': {
accounts: [],
},
wallet: {
accounts: [],
},
},
sessionProperties: {},
isMultichainOrigin: true,
};
const result = removeScope(caveatValue, 'eip155:5');
expect(result).toStrictEqual({
operation: CaveatMutatorOperation.RevokePermission,
});
});

it('does nothing if the target scope does not exist but the permission only has wallet scopes', () => {
const caveatValue = {
requiredScopes: {},
optionalScopes: {
'wallet:eip155': {
accounts: [],
},
wallet: {
accounts: [],
},
},
sessionProperties: {},
isMultichainOrigin: true,
};
const result = removeScope(caveatValue, 'eip155:5');
expect(result).toStrictEqual({
operation: CaveatMutatorOperation.Noop,
});
});

it('does nothing if the given scope is not found in either requiredScopes or optionalScopes', () => {
const caveatValue = {
requiredScopes: {
'eip155:1': {
accounts: [],
Expand All @@ -168,16 +211,16 @@ describe('caip25EndowmentBuilder', () => {
sessionProperties: {},
isMultichainOrigin: true,
};
const result = removeScope(ethereumGoerliCaveat, 'eip155:2');
const result = removeScope(caveatValue, 'eip155:2');
expect(result).toStrictEqual({
operation: CaveatMutatorOperation.Noop,
});
});
});

describe('removeAccount', () => {
it('returns a version of the caveat with the given account removed from requiredScopes if it is present', () => {
const ethereumGoerliCaveat: Caip25CaveatValue = {
it('updates the caveat with the given account removed from requiredScopes if it is present', () => {
const caveatValue: Caip25CaveatValue = {
requiredScopes: {
'eip155:1': {
accounts: ['eip155:1:0x1', 'eip155:1:0x2'],
Expand All @@ -186,7 +229,7 @@ describe('caip25EndowmentBuilder', () => {
optionalScopes: {},
isMultichainOrigin: true,
};
const result = removeAccount(ethereumGoerliCaveat, '0x1');
const result = removeAccount(caveatValue, '0x1');
expect(result).toStrictEqual({
operation: CaveatMutatorOperation.UpdateValue,
value: {
Expand All @@ -201,8 +244,8 @@ describe('caip25EndowmentBuilder', () => {
});
});

it('returns a version of the caveat with the given account removed from optionalScopes if it is present', () => {
const ethereumGoerliCaveat: Caip25CaveatValue = {
it('updates the caveat with the given account removed from optionalScopes if it is present', () => {
const caveatValue: Caip25CaveatValue = {
requiredScopes: {},
optionalScopes: {
'eip155:1': {
Expand All @@ -211,7 +254,7 @@ describe('caip25EndowmentBuilder', () => {
},
isMultichainOrigin: true,
};
const result = removeAccount(ethereumGoerliCaveat, '0x1');
const result = removeAccount(caveatValue, '0x1');
expect(result).toStrictEqual({
operation: CaveatMutatorOperation.UpdateValue,
value: {
Expand All @@ -226,8 +269,8 @@ describe('caip25EndowmentBuilder', () => {
});
});

it('returns a version of the caveat with the given account removed from requiredScopes and optionalScopes if it is present', () => {
const ethereumGoerliCaveat: Caip25CaveatValue = {
it('updates the caveat with the given account removed from requiredScopes and optionalScopes if it is present', () => {
const caveatValue: Caip25CaveatValue = {
requiredScopes: {
'eip155:1': {
accounts: ['eip155:1:0x1', 'eip155:1:0x2'],
Expand All @@ -243,7 +286,7 @@ describe('caip25EndowmentBuilder', () => {
},
isMultichainOrigin: true,
};
const result = removeAccount(ethereumGoerliCaveat, '0x1');
const result = removeAccount(caveatValue, '0x1');
expect(result).toStrictEqual({
operation: CaveatMutatorOperation.UpdateValue,
value: {
Expand All @@ -265,8 +308,71 @@ describe('caip25EndowmentBuilder', () => {
});
});

it('returns the caveat unchanged when the given account is not found in either requiredScopes or optionalScopes', () => {
const ethereumGoerliCaveat: Caip25CaveatValue = {
it('revokes the permission if the only account is removed', () => {
jiexi marked this conversation as resolved.
Show resolved Hide resolved
const caveatValue: Caip25CaveatValue = {
requiredScopes: {},
optionalScopes: {
'eip155:1': {
accounts: ['eip155:1:0x1'],
},
},
isMultichainOrigin: true,
};
const result = removeAccount(caveatValue, '0x1');
expect(result).toStrictEqual({
operation: CaveatMutatorOperation.RevokePermission,
});
});

it('updates the permission with the target account removed if the target account does exist and `wallet:eip155` is the only scope with remaining accounts after', () => {
const caveatValue: Caip25CaveatValue = {
requiredScopes: {},
optionalScopes: {
'eip155:1': {
accounts: ['eip155:1:0x1'],
},
'wallet:eip155': {
accounts: ['wallet:eip155:0x1', 'wallet:eip155:0x2'],
},
},
isMultichainOrigin: true,
};
const result = removeAccount(caveatValue, '0x1');
expect(result).toStrictEqual({
operation: CaveatMutatorOperation.UpdateValue,
value: {
requiredScopes: {},
optionalScopes: {
'eip155:1': {
accounts: [],
},
'wallet:eip155': {
accounts: ['wallet:eip155:0x2'],
},
},
isMultichainOrigin: true,
},
});
});

it('does nothing if the target account does not exist but the permission already has no accounts', () => {
const caveatValue: Caip25CaveatValue = {
requiredScopes: {},
optionalScopes: {
'eip155:1': {
accounts: [],
},
},
isMultichainOrigin: true,
};
const result = removeAccount(caveatValue, '0x1');
expect(result).toStrictEqual({
operation: CaveatMutatorOperation.Noop,
});
});

it('does nothing if the given account is not found in either requiredScopes or optionalScopes', () => {
const caveatValue: Caip25CaveatValue = {
requiredScopes: {
'eip155:1': {
accounts: ['eip155:1:0x1', 'eip155:1:0x2'],
Expand All @@ -279,7 +385,7 @@ describe('caip25EndowmentBuilder', () => {
},
isMultichainOrigin: true,
};
const result = removeAccount(ethereumGoerliCaveat, '0x3');
const result = removeAccount(caveatValue, '0x3');
expect(result).toStrictEqual({
operation: CaveatMutatorOperation.Noop,
});
Expand Down
Loading
Loading