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

Conflicting Sync error. #857

Merged
merged 3 commits into from
Aug 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
8 changes: 8 additions & 0 deletions .changeset/tall-birds-dress.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
---
"@web5/agent": patch
"@web5/identity-agent": patch
"@web5/proxy-agent": patch
"@web5/user-agent": patch
---

Sync vs StartSync conflicting error.
20 changes: 19 additions & 1 deletion packages/agent/src/connect.ts
Original file line number Diff line number Diff line change
Expand Up @@ -185,9 +185,27 @@ export type ConnectPermissionRequest = {
permissionScopes: DwnPermissionScope[];
};

/**
* Shorthand for the types of permissions that can be requested.
*/
export type Permission = 'write' | 'read' | 'delete' | 'query' | 'subscribe';

function createPermissionRequestForProtocol(definition: DwnProtocolDefinition, permissions: Permission[]): ConnectPermissionRequest {
/**
* The options for creating a permission request for a given protocol.
*/
export type ProtocolPermissionOptions = {
/** The protocol definition for the protocol being requested */
definition: DwnProtocolDefinition;

/** The permissions being requested for the protocol */
permissions: Permission[];
};

/**
* Creates a set of Dwn Permission Scopes to request for a given protocol.
* If no permissions are provided, the default is to request all permissions (write, read, delete, query, subscribe).
*/
function createPermissionRequestForProtocol({ definition, permissions }: ProtocolPermissionOptions): ConnectPermissionRequest {
const requests: DwnPermissionScope[] = [];

// In order to enable sync, we must request permissions for `MessagesQuery`, `MessagesRead` and `MessagesSubscribe`
Expand Down
3 changes: 2 additions & 1 deletion packages/agent/src/sync-engine-level.ts
Original file line number Diff line number Diff line change
Expand Up @@ -288,7 +288,8 @@ export class SyncEngineLevel implements SyncEngine {
}

try {
await this.sync();
await this.push();
await this.pull();
} catch (error: any) {
this.stopSync();
reject(error);
Expand Down
12 changes: 9 additions & 3 deletions packages/agent/tests/connect.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -822,7 +822,9 @@ describe('web5 connect', function () {
}
};

const permissionRequests = WalletConnect.createPermissionRequestForProtocol(protocol, []);
const permissionRequests = WalletConnect.createPermissionRequestForProtocol({
definition: protocol, permissions: []
});

expect(permissionRequests.protocolDefinition).to.deep.equal(protocol);
expect(permissionRequests.permissionScopes.length).to.equal(3); // only includes the sync permissions
Expand All @@ -846,7 +848,9 @@ describe('web5 connect', function () {
}
};

const permissionRequests = WalletConnect.createPermissionRequestForProtocol(protocol, ['write', 'read']);
const permissionRequests = WalletConnect.createPermissionRequestForProtocol({
definition: protocol, permissions: ['write', 'read']
});

expect(permissionRequests.protocolDefinition).to.deep.equal(protocol);

Expand All @@ -871,7 +875,9 @@ describe('web5 connect', function () {
}
};

const permissionRequests = WalletConnect.createPermissionRequestForProtocol(protocol, ['write', 'read', 'delete', 'query', 'subscribe']);
const permissionRequests = WalletConnect.createPermissionRequestForProtocol({
definition: protocol, permissions: ['write', 'read', 'delete', 'query', 'subscribe']
});

expect(permissionRequests.protocolDefinition).to.deep.equal(protocol);

Expand Down
31 changes: 21 additions & 10 deletions packages/agent/tests/sync-engine-level.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2415,23 +2415,28 @@ describe('SyncEngineLevel', () => {
});

describe('startSync()', () => {
it('calls sync() in each interval', async () => {
it('calls pull() and push() in each interval', async () => {
await testHarness.agent.sync.registerIdentity({
did: alice.did.uri,
});

const syncSpy = sinon.stub(SyncEngineLevel.prototype, 'sync');
syncSpy.resolves();
const pullSpy = sinon.stub(SyncEngineLevel.prototype as any, 'pull');
pullSpy.resolves();

const pushSpy = sinon.stub(SyncEngineLevel.prototype as any, 'push');
pushSpy.resolves();

const clock = sinon.useFakeTimers();

testHarness.agent.sync.startSync({ interval: '500ms' });

await clock.tickAsync(1_400); // just under 3 intervals
syncSpy.restore();
pullSpy.restore();
pushSpy.restore();
clock.restore();

expect(syncSpy.callCount).to.equal(2, 'push');
expect(pullSpy.callCount).to.equal(2, 'push');
expect(pushSpy.callCount).to.equal(2, 'pull');
});

it('does not call sync() again until a sync round finishes', async () => {
Expand All @@ -2441,24 +2446,30 @@ describe('SyncEngineLevel', () => {

const clock = sinon.useFakeTimers();

const syncSpy = sinon.stub(SyncEngineLevel.prototype, 'sync');
syncSpy.returns(new Promise((resolve) => {
const pullSpy = sinon.stub(SyncEngineLevel.prototype as any, 'pull');
pullSpy.returns(new Promise<void>((resolve) => {
clock.setTimeout(() => {
resolve();
}, 1_500); // more than the interval
}));

const pushSpy = sinon.stub(SyncEngineLevel.prototype as any, 'push');
pushSpy.resolves();

testHarness.agent.sync.startSync({ interval: '500ms' });

await clock.tickAsync(1_400); // less time than the push

expect(syncSpy.callCount).to.equal(1, 'sync');
expect(pullSpy.callCount).to.equal(1, 'pull');
expect(pullSpy.callCount).to.equal(1, 'push');

await clock.tickAsync(600); //remaining time for a 2nd sync

expect(syncSpy.callCount).to.equal(2, 'sync');
expect(pullSpy.callCount).to.equal(2, 'pull');
expect(pushSpy.callCount).to.equal(2, 'push');

syncSpy.restore();
pullSpy.restore();
pushSpy.restore();
clock.restore();
});
});
Expand Down
25 changes: 22 additions & 3 deletions packages/api/src/web5.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,12 +36,28 @@ export type DidCreateOptions = {
dwnEndpoints?: string[];
}

/**
* Represents a permission request for a protocol definition.
*/
export type ConnectPermissionRequest = {
/**
* The protocol definition for the protocol being requested.
*/
protocolDefinition: DwnProtocolDefinition;
/**
* The permissions being requested for the protocol. If none are provided, the default is to request all permissions.
*/
permissions?: Permission[];
}

/**
* Options for connecting to a Web5 agent. This includes the ability to connect to an external wallet
*/
export type ConnectOptions = Omit<WalletConnectOptions, 'permissionRequests'> & {
/**
* The permissions that are being requested for the connected DID.
* This is used to create the {@link ConnectPermissionRequest} for the wallet connect flow.
*/
permissionRequests: ConnectPermissionRequest[];
}

Expand Down Expand Up @@ -286,9 +302,12 @@ export class Web5 {
// No connected identity found and connectOptions are provided, attempt to import a delegated DID from an external wallet
try {
const { permissionRequests, ...connectOptions } = walletConnectOptions;
const walletPermissionRequests = permissionRequests.map(({ protocolDefinition, permissions }) => WalletConnect.createPermissionRequestForProtocol(protocolDefinition, permissions ?? [
'read', 'write', 'delete', 'query', 'subscribe'
]));
const walletPermissionRequests = permissionRequests.map(({ protocolDefinition, permissions }) => WalletConnect.createPermissionRequestForProtocol({
definition : protocolDefinition,
permissions : permissions ?? [
'read', 'write', 'delete', 'query', 'subscribe'
]}
));

const { delegatePortableDid, connectedDid, delegateGrants } = await WalletConnect.initClient({
...connectOptions,
Expand Down
6 changes: 3 additions & 3 deletions packages/api/tests/web5.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -846,7 +846,7 @@ describe('web5 api', () => {
const call = requestPermissionsSpy.getCall(0);

// since no explicit permissions were provided, all permissions should be requested
expect(call.args[1]).to.have.members([
expect(call.args[0].permissions).to.have.members([
'read', 'write', 'delete', 'query', 'subscribe'
]);
}
Expand Down Expand Up @@ -920,14 +920,14 @@ describe('web5 api', () => {
const call1 = requestPermissionsSpy.getCall(0);

// since no explicit permissions were provided for the first protocol, all permissions should be requested
expect(call1.args[1]).to.have.members([
expect(call1.args[0].permissions).to.have.members([
'read', 'write', 'delete', 'query', 'subscribe'
]);

const call2 = requestPermissionsSpy.getCall(1);

// only the provided permissions should be requested for the second protocol
expect(call2.args[1]).to.have.members([
expect(call2.args[0].permissions).to.have.members([
'read', 'write'
]);
}
Expand Down
Loading