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

[Push, Echo] api milestone 1 #624

Merged
merged 42 commits into from
Dec 8, 2022
Merged

[Push, Echo] api milestone 1 #624

merged 42 commits into from
Dec 8, 2022

Conversation

llbartekll
Copy link
Contributor

@llbartekll llbartekll commented Dec 5, 2022

  1. Add Echo target - component for registering a client at Echo server
  • has not been e2e tested yet, there is still issue with echo server
  1. Push API
  • Add Push subscription negotiation
  • Add Wallet and Dapp clients - with not all methods implemented
  • allows to send push message over relay network
  • integration tests testDappSendsPushMessage and testWalletApprovesPushRequest pass
  1. There is push registration pulled from Derek's PR, it will be replaced with Echo SDK registration when issue on Echo server is fixed.

@llbartekll llbartekll changed the base branch from main to develop December 5, 2022 13:17
@arein arein added the accepted label Dec 5, 2022
@llbartekll llbartekll marked this pull request as draft December 5, 2022 13:17
@llbartekll llbartekll changed the title Push api milestone 1 [Push, Echo] api milestone 1 Dec 6, 2022
Copy link
Member

@arein arein left a comment

Choose a reason for hiding this comment

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

Looks good high level. Important IMO that it works, too with the simulator but I think that works

@llbartekll llbartekll marked this pull request as ready for review December 6, 2022 14:41
…V2 into push-api-milestone-1

# Conflicts:
#	Example/ExampleApp.xcodeproj/project.pbxproj
#	Example/IntegrationTests/Pairing/PairingTests.swift
wait(for: [expectation], timeout: InputConfig.defaultTimeout)
}

func testWalletRejectsPushRequest() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Planning to implement in this PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes

Copy link
Contributor Author

Choose a reason for hiding this comment

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

but it is not implemented in the engine yet

}

private func generateAgreementKeys(peerPublicKeyHex: String, selfpublicKeyHex: String) throws -> (topic: String, keys: AgreementKeys) {
let selfPublicKey = try AgreementPublicKey(hex: selfpublicKeyHex)
Copy link
Contributor

Choose a reason for hiding this comment

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

just an idea. We doing same actions to generate keys, topic, setAgreementSecret in many places. May be we should move it in KMS package?

Copy link
Contributor Author

@llbartekll llbartekll Dec 7, 2022

Choose a reason for hiding this comment

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

maybe worth to consider, should we open an issue?

Copy link
Contributor

Choose a reason for hiding this comment

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

lets open


import Foundation

public struct PushMessage: Codable {
Copy link
Contributor

Choose a reason for hiding this comment

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

All params is required?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

so far, yes but it may be a good suggestion to change specs

Copy link
Contributor

@flypaper0 flypaper0 left a comment

Choose a reason for hiding this comment

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

lgtm, just few comments

@llbartekll llbartekll merged commit dff1741 into develop Dec 8, 2022
@llbartekll llbartekll deleted the push-api-milestone-1 branch December 8, 2022 11:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants