Skip to content

Commit eda8860

Browse files
authored
Fix issue where accountMeta was not being included on the response object correctly. (#95)
* Fix issue where accountMeta was not being included on the response object correctly. Fix test that was asserting that the accountMeta was incorrectly included on the response object. * Improve accountMeta - accountMeta is now a required field in the response - simplified resolving accountMeta in the orchestrator - added resolving environment to existing Promise.all statement to reduce latency - remove errant console.log from dapp
1 parent 3799b19 commit eda8860

6 files changed

Lines changed: 39 additions & 33 deletions

File tree

packages/gator-permissions-snap/snap.manifest.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
"url": "https://github.com/MetaMask/snap-7715-permissions.git"
88
},
99
"source": {
10-
"shasum": "D1xZgkqOmv2yug3z6/YsbDkUWAzgxWujvWgz+W5bWH8=",
10+
"shasum": "B/45l1lyLyqYX2GkYxvKAmCCWOCSu030Do+IEMdQV+c=",
1111
"location": {
1212
"npm": {
1313
"filePath": "dist/bundle.js",

packages/gator-permissions-snap/src/core/permissionRequestLifecycleOrchestrator.ts

Lines changed: 29 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import type {
2+
AccountMeta,
23
PermissionRequest,
34
PermissionResponse,
45
} from '@metamask/7715-permissions-shared/types';
@@ -203,21 +204,21 @@ export class PermissionRequestLifecycleOrchestrator {
203204
isAdjustmentAllowed,
204205
};
205206

206-
const [address, accountMeta, delegationManager] = await Promise.all([
207-
this.#accountController.getAccountAddress({
208-
chainId,
209-
}),
210-
this.#accountController.getAccountMetadata({
211-
chainId,
212-
}),
213-
this.#accountController.getDelegationManager({
214-
chainId,
215-
}),
216-
]);
217-
218-
const environment = await this.#accountController.getEnvironment({
219-
chainId,
220-
});
207+
const [environment, address, accountMetadata, delegationManager] =
208+
await Promise.all([
209+
this.#accountController.getEnvironment({
210+
chainId,
211+
}),
212+
this.#accountController.getAccountAddress({
213+
chainId,
214+
}),
215+
this.#accountController.getAccountMetadata({
216+
chainId,
217+
}),
218+
this.#accountController.getDelegationManager({
219+
chainId,
220+
}),
221+
]);
221222

222223
const caveatBuilder = await lifecycleHandlers.appendCaveats({
223224
permission: populatedPermission,
@@ -241,23 +242,25 @@ export class PermissionRequestLifecycleOrchestrator {
241242
caveats: finalCaveatBuilder,
242243
}),
243244
});
244-
const permissionsContext = encodeDelegation([signedDelegation]);
245245

246-
// we do this because we cannot have undefined properties set on the response
247-
const accountMetaObject =
248-
accountMeta.factory && accountMeta.factoryData
249-
? {
250-
factory: accountMeta.factory,
251-
factoryData: accountMeta.factoryData,
252-
}
253-
: {};
246+
const context = encodeDelegation([signedDelegation]);
247+
248+
const accountMeta: AccountMeta[] =
249+
accountMetadata.factory && accountMetadata.factoryData
250+
? [
251+
{
252+
factory: accountMetadata.factory,
253+
factoryData: accountMetadata.factoryData,
254+
},
255+
]
256+
: [];
254257

255258
const response: PermissionResponse = {
256259
...grantedPermissionRequest,
257260
chainId: toHex(chainId),
258261
address,
259-
context: permissionsContext,
260-
...accountMetaObject,
262+
accountMeta,
263+
context,
261264
signerMeta: {
262265
delegationManager,
263266
},

packages/gator-permissions-snap/test/core/permissionRequestLifecycleOrchestrator.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,7 @@ describe('PermissionRequestLifecycleOrchestrator', () => {
173173
expect(result.approved).toBe(true);
174174
expect(result.approved && result.response).toStrictEqual({
175175
...mockPermissionRequest,
176-
...mockAccountMetadata,
176+
accountMeta: [mockAccountMetadata],
177177
permission: mockPopulatedPermission,
178178
address: grantingAccountAddress,
179179
context: expect.stringMatching(/^0x[0-9a-fA-F]+$/u),

packages/gator-permissions-snap/test/rpc/rpcHandler.test.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ const VALID_PERMISSION_RESPONSE: PermissionResponse = {
4848
data: { justification: 'Testing permission request' },
4949
},
5050
context: TEST_CONTEXT,
51+
accountMeta: [],
5152
signerMeta: {
5253
delegationManager: TEST_ADDRESS,
5354
},

packages/permissions-kernel-snap/snap.manifest.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
"url": "https://github.com/MetaMask/snap-7715-permissions.git"
88
},
99
"source": {
10-
"shasum": "fgjg6Gg3LrnSgdKeWo8X3v3vg3fK4lDl728FzSySqPU=",
10+
"shasum": "VvmzRf0CQohjyCGxQipja9d4Ghoq1q9w5kAJryTTw/8=",
1111
"location": {
1212
"npm": {
1313
"filePath": "dist/bundle.js",

packages/shared/src/types/7715-permissions-response.ts

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,11 +18,13 @@ export const zGrantedPermission = z.object({
1818
context: zHexStr,
1919

2020
/**
21-
* Optional but when present then fields for factory and factoryData are required as defined in ERC-4337.
22-
* They are either both specified, or none.
23-
* DApp MUST deploy the accounts by calling the factory contract with factoryData as the calldata.
21+
* The accountMeta field is required and contains information needed to deploy accounts.
22+
* Each entry specifies a factory contract and its associated deployment data.
23+
* If no account deployment is needed when redeeming the permission, this array must be empty.
24+
* When non-empty, DApps MUST deploy the accounts by calling the factory contract with factoryData as the calldata.
25+
* Defined in ERC-4337.
2426
*/
25-
accountMeta: z.array(zAccountMeta).optional(),
27+
accountMeta: z.array(zAccountMeta),
2628

2729
/**
2830
* Account to assign the permissions to and is dependent on the account type.

0 commit comments

Comments
 (0)