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

[ABW-4001] Update PreAuthorization models #1400

Merged
merged 6 commits into from
Nov 29, 2024
Merged
Show file tree
Hide file tree
Changes from 3 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
2 changes: 1 addition & 1 deletion RadixWallet.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -9279,7 +9279,7 @@
repositoryURL = "https://github.com/radixdlt/sargon";
requirement = {
kind = exactVersion;
version = 1.1.64;
version = 1.1.65;
};
};
/* End XCRemoteSwiftPackageReference section */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -114,8 +114,8 @@
"kind" : "remoteSourceControl",
"location" : "https://github.com/radixdlt/sargon",
"state" : {
"revision" : "7ca859a59155eea571d7ec725db04dfc9520c391",
"version" : "1.1.64"
"revision" : "4f1e6a35914aa6d9bb291b11f6eae33a7bdbc0d5",
"version" : "1.1.65"
}
},
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,6 @@ extension PreAuthorizationClient {

struct PollStatusRequest: Sendable {
let subintentHash: SubintentHash
let expiration: DappToWalletInteractionSubintentExpiration
let expirationTimestamp: Date
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ extension PreAuthorizationClient: DependencyKey {
}

let pollStatus: PollStatus = { request in
try await SargonOS.shared.pollPreAuthorizationStatus(intentHash: request.subintentHash, expiration: request.expiration)
try await SargonOS.shared.pollPreAuthorizationStatus(intentHash: request.subintentHash, expirationTimestamp: request.expirationTimestamp)
}

return Self(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ struct DappInteractionCoordinator: Sendable, FeatureReducer {
}

enum DelegateAction: Sendable, Equatable {
case submit(WalletToDappInteractionResponse, DappMetadata, PreAuthorizationData? = nil)
case submit(WalletToDappInteractionResponse, DappMetadata)
case dismiss(DappMetadata, DappInteractionCompletionKind)
case dismissSilently
}
Expand Down Expand Up @@ -102,8 +102,8 @@ struct DappInteractionCoordinator: Sendable, FeatureReducer {
case .flow(.delegate(.dismiss)):
return .send(.delegate(.dismissSilently))

case let .flow(.delegate(.submit(response, dappMetadata, preAuthData))):
return .send(.delegate(.submit(.success(response), dappMetadata, preAuthData)))
case let .flow(.delegate(.submit(response, dappMetadata))):
return .send(.delegate(.submit(.success(response), dappMetadata)))

default:
return .none
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ struct DappInteractionFlow: Sendable, FeatureReducer {
enum DelegateAction: Sendable, Equatable {
case dismissWithFailure(WalletToDappInteractionFailureResponse)
case dismissWithSuccess(DappMetadata, DappInteractionCompletionKind)
case submit(WalletToDappInteractionSuccessResponse, DappMetadata, PreAuthorizationData? = nil)
case submit(WalletToDappInteractionSuccessResponse, DappMetadata)
case dismiss
}

Expand Down Expand Up @@ -515,10 +515,10 @@ extension DappInteractionFlow {
_ signedSubintent: SignedSubintent,
_ expiration: DappToWalletInteractionSubintentExpiration
) -> Effect<Action> {
let preAuthResponse = newWalletToDappInteractionPreAuthorizationResponseItems(signedSubintent: signedSubintent)
let preAuthResponse = newWalletToDappInteractionPreAuthorizationResponseItems(signedSubintent: signedSubintent, expirationTimestamp: expiration.timestamp)
GhenadieVP marked this conversation as resolved.
Show resolved Hide resolved
state.responseItems[item] = .remote(.preAuthorization(preAuthResponse))

return continueEffect(for: &state, preAuthData: .init(subintentHash: signedSubintent.subintent.hash(), expiration: expiration))
return continueEffect(for: &state)
}

func handlePreAuthorizationFailure(
Expand Down Expand Up @@ -746,7 +746,7 @@ extension DappInteractionFlow {
}
}

func continueEffect(for state: inout State, preAuthData: PreAuthorizationData? = nil) -> Effect<Action> {
func continueEffect(for state: inout State) -> Effect<Action> {
if
let nextRequest = state.interactionItems.first(where: { state.responseItems[$0] == nil }),
let destination = Path.State(
Expand All @@ -768,11 +768,11 @@ extension DappInteractionFlow {
}
return .none
} else {
return finishInteractionFlow(state, preAuthData: preAuthData)
return finishInteractionFlow(state)
}
}

func finishInteractionFlow(_ state: State, preAuthData: PreAuthorizationData?) -> Effect<Action> {
func finishInteractionFlow(_ state: State) -> Effect<Action> {
guard let response = WalletToDappInteractionSuccessResponse(
for: state.remoteInteraction,
with: state.responseItems.values.compactMap(/State.AnyInteractionResponseItem.remote)
Expand All @@ -796,7 +796,7 @@ extension DappInteractionFlow {
}
}

await send(.delegate(.submit(response, state.dappMetadata, preAuthData)))
await send(.delegate(.submit(response, state.dappMetadata)))
}
}

Expand Down Expand Up @@ -1156,3 +1156,14 @@ struct SavedPersonaDataInPersonaDoesNotMatchWalletInteractionResponseItem: Swift
struct PersonaDataEntryNotFoundInResponse: Swift.Error {
let kind: PersonaData.Entry.Kind
}

private extension DappToWalletInteractionSubintentExpiration {
matiasbzurovski marked this conversation as resolved.
Show resolved Hide resolved
var timestamp: Date {
switch self {
case let .afterDelay(afterDelay):
Date().addingTimeInterval(TimeInterval(afterDelay.expireAfterSeconds))
Copy link
Contributor

Choose a reason for hiding this comment

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

Date()? Shouldn't the timestamp be UTC?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh dearful dates and timezones.. Let me write down my dummy/cautious reasoning to validate the current logic makes sense.

In this case, the subintent will expire X seconds after signature. Let's use 600 (10 minutes) for this example, which we will set using the test dApp.

It is currently 13:54 PM GMT+1 in Madrid. This means that when I get to this state, the timestamp: Date will return 14:04 GMT+1.

Given this timestamp is used on the PollPreAuthorizationStatus+View, we can validate that after signing the preAuth, the view says that it expires in 9 minutes (it shows 10 minutes only for a second) and not in -50 minutes (as it would be in GMT timezone)

So far so good, now we need to make sure that the value sent to Sargon and then to the dApp makes sense. The process here is:

  • convert Swift.Date into Sargon.Timestamp (happens thanks to magic code in uniffi.toml)
  • get amount of second since this timestamp and send it to dApp (done here)

Now, if we check the final result sent to the dApp we can see it is 1732799073.
Screenshot 2024-11-28 at 13 58 36

Then, using a Playground I can confirm this value corresponds to 14:04 in my timezone:
Screenshot 2024-11-28 at 14 00 09

Or if I use a helper tool such as https://www.unixtimestamp.com/ we can verify it is 14:04 GMT+1 and 13:04 GMT:

Copy link
Contributor

@GhenadieVP GhenadieVP Nov 28, 2024

Choose a reason for hiding this comment

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

given that the same delay applies, regardless of the base, I think this could be safe, unless user goes and changes the date in the settings while the countdown is ticking 😄.

But as we have discussed, the same timestamp that is calculated when creating the Subintent should be used in all places.

case let .atTime(atTime):
atTime.date
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,7 @@ import SwiftUI

typealias RequestEnvelope = DappInteractionClient.RequestEnvelope

// MARK: - PreAuthorizationData
struct PreAuthorizationData: Sendable, Hashable {
let subintentHash: SubintentHash
let expiration: DappToWalletInteractionSubintentExpiration
}

// MARK: - RequestEnvelope + Identifiable
// MARK: Identifiable
extension RequestEnvelope: Identifiable {
typealias ID = WalletInteractionId
var id: ID {
Expand Down Expand Up @@ -49,15 +43,13 @@ struct DappInteractor: Sendable, FeatureReducer {
case sentResponseToDapp(
WalletToDappInteractionResponse,
for: RequestEnvelope,
DappMetadata,
PreAuthorizationData?
DappMetadata
)
case failedToSendResponseToDapp(
WalletToDappInteractionResponse,
for: RequestEnvelope,
DappMetadata,
reason: String,
preAuthData: PreAuthorizationData?
reason: String
)
case presentResponseSuccessView(DappMetadata, DappInteractionCompletionKind, P2P.Route)
case presentInvalidRequest(
Expand Down Expand Up @@ -86,7 +78,7 @@ struct DappInteractor: Sendable, FeatureReducer {

enum ResponseFailure: Sendable, Hashable {
case cancelButtonTapped(RequestEnvelope)
case retryButtonTapped(WalletToDappInteractionResponse, for: RequestEnvelope, DappMetadata, PreAuthorizationData?)
case retryButtonTapped(WalletToDappInteractionResponse, for: RequestEnvelope, DappMetadata)
}

enum InvalidRequest: Sendable, Hashable {
Expand Down Expand Up @@ -168,12 +160,12 @@ struct DappInteractor: Sendable, FeatureReducer {
case .presentQueuedRequestIfNeeded:
return presentQueuedRequestIfNeededEffect(for: &state)

case let .sentResponseToDapp(response, for: request, dappMetadata, preAuthData):
case let .sentResponseToDapp(response, for: request, dappMetadata):
switch response {
case .success:
if let preAuthData {
case let .success(success):
if let response = success.preAuthorizationResponse {
dismissCurrentModalAndRequest(request, for: &state, clearDappInteraction: false)
return pollPreAuthorizationEffect(for: &state, request: request, dappMetadata: dappMetadata, preAuthData: preAuthData)
return pollPreAuthorizationEffect(for: &state, request: request, dappMetadata: dappMetadata, response: response)
} else {
dismissCurrentModalAndRequest(request, for: &state)
return .send(.internal(.presentResponseSuccessView(dappMetadata, .personaData, request.route)))
Expand All @@ -183,15 +175,15 @@ struct DappInteractor: Sendable, FeatureReducer {
return delayedMediumEffect(internal: .presentQueuedRequestIfNeeded)
}

case let .failedToSendResponseToDapp(response, for: request, dappMetadata, reason, preAuthData):
case let .failedToSendResponseToDapp(response, for: request, dappMetadata, reason):
dismissCurrentModalAndRequest(request, for: &state)
state.destination = .responseFailure(.init(
title: { TextState(L10n.Common.errorAlertTitle) },
actions: {
ButtonState(role: .cancel, action: .cancelButtonTapped(request)) {
TextState(L10n.Common.cancel)
}
ButtonState(action: .retryButtonTapped(response, for: request, dappMetadata, preAuthData)) {
ButtonState(action: .retryButtonTapped(response, for: request, dappMetadata)) {
TextState(L10n.Common.retry)
}
},
Expand Down Expand Up @@ -251,8 +243,8 @@ struct DappInteractor: Sendable, FeatureReducer {
let request = dappInteraction.request

switch delegateAction {
case let .submit(responseToDapp, dappMetadata, preAuthData):
return sendResponseToDappEffect(responseToDapp, for: request, dappMetadata: dappMetadata, preAuthData: preAuthData)
case let .submit(responseToDapp, dappMetadata):
return sendResponseToDappEffect(responseToDapp, for: request, dappMetadata: dappMetadata)
case let .dismiss(dappMetadata, txID):
dismissCurrentModalAndRequest(request, for: &state)
return delayedShortEffect(for: .internal(.presentResponseSuccessView(dappMetadata, txID, request.route)))
Expand All @@ -276,8 +268,8 @@ struct DappInteractor: Sendable, FeatureReducer {
case let .cancelButtonTapped(request):
dismissCurrentModalAndRequest(request, for: &state)
return .send(.internal(.presentQueuedRequestIfNeeded))
case let .retryButtonTapped(response, request, dappMetadata, preAuthData):
return sendResponseToDappEffect(response, for: request, dappMetadata: dappMetadata, preAuthData: preAuthData)
case let .retryButtonTapped(response, request, dappMetadata):
return sendResponseToDappEffect(response, for: request, dappMetadata: dappMetadata)
}

case let .invalidRequest(action):
Expand Down Expand Up @@ -333,8 +325,7 @@ struct DappInteractor: Sendable, FeatureReducer {
func sendResponseToDappEffect(
_ responseToDapp: WalletToDappInteractionResponse,
for request: RequestEnvelope,
dappMetadata: DappMetadata,
preAuthData: PreAuthorizationData?
dappMetadata: DappMetadata
) -> Effect<Action> {
.run { send in

Expand All @@ -359,8 +350,7 @@ struct DappInteractor: Sendable, FeatureReducer {
.sentResponseToDapp(
responseToDapp,
for: request,
dappMetadata,
preAuthData
dappMetadata
)
))
} else {
Expand All @@ -373,8 +363,7 @@ struct DappInteractor: Sendable, FeatureReducer {
responseToDapp,
for: request,
dappMetadata,
reason: error.localizedDescription,
preAuthData: preAuthData
reason: error.localizedDescription
)
))
} else {
Expand All @@ -401,13 +390,13 @@ struct DappInteractor: Sendable, FeatureReducer {
for state: inout State,
request: RequestEnvelope,
dappMetadata: DappMetadata,
preAuthData: PreAuthorizationData
response: WalletToDappInteractionSubintentResponseItem
) -> Effect<Action> {
state.destination = .pollPreAuthorizationStatus(
.init(
dAppMetadata: dappMetadata,
subintentHash: preAuthData.subintentHash,
expiration: preAuthData.expiration,
subintentHash: response.signedSubintent.subintent.hash(),
expirationTimestamp: response.expirationTimestamp,
isDeepLink: request.route.isDeepLink,
request: request
)
Expand Down Expand Up @@ -572,3 +561,14 @@ private extension DappInteractionCompletionKind {
}
}
}

private extension WalletToDappInteractionSuccessResponse {
var preAuthorizationResponse: WalletToDappInteractionSubintentResponseItem? {
switch items {
case let .preAuthorization(preAuthorization):
preAuthorization.response
case .authorizedRequest, .unauthorizedRequest, .transaction:
nil
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ struct PollPreAuthorizationStatus: Sendable, FeatureReducer {
struct State: Sendable, Hashable {
let dAppMetadata: DappMetadata
let subintentHash: SubintentHash
let expiration: Expiration
let expirationTimestamp: Date
let isDeepLink: Bool
let request: RequestEnvelope
var status = Status.unknown
Expand All @@ -14,21 +14,16 @@ struct PollPreAuthorizationStatus: Sendable, FeatureReducer {
init(
dAppMetadata: DappMetadata,
subintentHash: SubintentHash,
expiration: Expiration,
expirationTimestamp: Date,
isDeepLink: Bool,
request: RequestEnvelope
) {
self.dAppMetadata = dAppMetadata
self.subintentHash = subintentHash
self.expiration = expiration
self.expirationTimestamp = expirationTimestamp
self.isDeepLink = isDeepLink
self.request = request
switch expiration {
case let .afterDelay(afterDelay):
secondsToExpiration = Int(afterDelay.expireAfterSeconds)
case let .atTime(atTime):
secondsToExpiration = Int(atTime.date.timeIntervalSinceNow)
}
self.secondsToExpiration = Int(expirationTimestamp.timeIntervalSinceNow)
}
}

Expand Down Expand Up @@ -79,7 +74,7 @@ struct PollPreAuthorizationStatus: Sendable, FeatureReducer {
private func pollStatus(state: inout State) -> Effect<Action> {
let request = PreAuthorizationClient.PollStatusRequest(
subintentHash: state.subintentHash,
expiration: state.expiration
expirationTimestamp: state.expirationTimestamp
)
return .run { send in
let status = try await preAuthorizationClient.pollStatus(request)
Expand Down
Loading