Skip to content

Commit

Permalink
[Bug fix] Prevent same Transaction from being commited twice. (#539)
Browse files Browse the repository at this point in the history
  • Loading branch information
CyonAlexRDX committed May 31, 2023
1 parent be94748 commit 4fd9b9a
Show file tree
Hide file tree
Showing 11 changed files with 50 additions and 14 deletions.
1 change: 1 addition & 0 deletions Sources/Clients/FaucetClient/FaucetClient+Live.swift
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ extension FaucetClient: DependencyKey {
.init(
networkID: networkID,
manifest: manifest,
nonce: engineToolkitClient.generateTXNonce(),
isFaucetTransaction: true,
ephemeralNotaryPublicKey: ephemeralNotary.publicKey
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,7 @@ public struct NotarizeTransactionResponse: Sendable, Hashable {
// MARK: - BuildTransactionIntentRequest
public struct BuildTransactionIntentRequest: Sendable {
public let networkID: NetworkID
public let nonce: Nonce
public let manifest: TransactionManifest
public let makeTransactionHeaderInput: MakeTransactionHeaderInput
public let isFaucetTransaction: Bool
Expand All @@ -151,12 +152,14 @@ public struct BuildTransactionIntentRequest: Sendable {
public init(
networkID: NetworkID,
manifest: TransactionManifest,
nonce: Nonce,
makeTransactionHeaderInput: MakeTransactionHeaderInput = .default,
isFaucetTransaction: Bool = false,
ephemeralNotaryPublicKey: Curve25519.Signing.PublicKey
) {
self.networkID = networkID
self.manifest = manifest
self.nonce = nonce
self.makeTransactionHeaderInput = makeTransactionHeaderInput
self.isFaucetTransaction = isFaucetTransaction
self.ephemeralNotaryPublicKey = ephemeralNotaryPublicKey
Expand Down Expand Up @@ -199,17 +202,20 @@ extension TransactionClient {
// MARK: - ManifestReviewRequest
public struct ManifestReviewRequest: Sendable {
public let manifestToSign: TransactionManifest
public let nonce: Nonce
public let feeToAdd: BigDecimal
public let makeTransactionHeaderInput: MakeTransactionHeaderInput
public let ephemeralNotaryPublicKey: Curve25519.Signing.PublicKey

public init(
manifestToSign: TransactionManifest,
nonce: Nonce,
makeTransactionHeaderInput: MakeTransactionHeaderInput = .default,
feeToAdd: BigDecimal,
ephemeralNotaryPublicKey: Curve25519.Signing.PublicKey = Curve25519.Signing.PrivateKey().publicKey
) {
self.manifestToSign = manifestToSign
self.nonce = nonce
self.feeToAdd = feeToAdd
self.makeTransactionHeaderInput = makeTransactionHeaderInput
self.ephemeralNotaryPublicKey = ephemeralNotaryPublicKey
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -232,12 +232,12 @@ extension TransactionClient {
let epoch = try await gatewayAPIClient.getEpoch()
let transactionSigners = try await getTransactionSigners(request)

let header = try TransactionHeader(
let header = TransactionHeader(
version: engineToolkitClient.getTransactionVersion(),
networkId: request.networkID,
startEpochInclusive: epoch,
endEpochExclusive: epoch + request.makeTransactionHeaderInput.epochWindow,
nonce: engineToolkitClient.generateTXNonce(),
nonce: request.nonce,
publicKey: SLIP10.PublicKey.eddsaEd25519(transactionSigners.notaryPublicKey).intoEngine(),
notaryAsSignatory: transactionSigners.notaryAsSignatory,
costUnitLimit: request.makeTransactionHeaderInput.costUnitLimit,
Expand Down Expand Up @@ -288,7 +288,7 @@ extension TransactionClient {
)
}

let getTransactionPreview: GetTransactionReview = { request in
let getTransactionReview: GetTransactionReview = { request in
let networkID = await gatewaysClient.getCurrentNetworkID()

let transactionPreviewRequest = try await createTransactionPreviewRequest(for: request, networkID: networkID)
Expand Down Expand Up @@ -325,6 +325,7 @@ extension TransactionClient {
let intent = try await buildTransactionIntent(.init(
networkID: gatewaysClient.getCurrentNetworkID(),
manifest: request.manifestToSign,
nonce: request.nonce,
makeTransactionHeaderInput: request.makeTransactionHeaderInput,
ephemeralNotaryPublicKey: request.ephemeralNotaryPublicKey
))
Expand Down Expand Up @@ -386,7 +387,7 @@ extension TransactionClient {
lockFeeWithSelectedPayer: lockFeeWithSelectedPayer,
addInstructionToManifest: addInstructionToManifest,
addGuaranteesToManifest: addGuaranteesToManifest,
getTransactionReview: getTransactionPreview,
getTransactionReview: getTransactionReview,
buildTransactionIntent: buildTransactionIntent,
notarizeTransaction: notarizeTransaction
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -129,10 +129,10 @@ public struct AccountPreferences: Sendable, FeatureReducer {
@Dependency(\.faucetClient) var faucetClient
@Dependency(\.errorQueue) var errorQueue
@Dependency(\.accountPortfoliosClient) var accountPortfoliosClient
@Dependency(\.engineToolkitClient) var engineToolkitClient

#if DEBUG
@Dependency(\.gatewayAPIClient) var gatewayAPIClient
@Dependency(\.engineToolkitClient) var engineToolkitClient
#endif // DEBUG

public init() {}
Expand Down Expand Up @@ -289,6 +289,7 @@ public struct AccountPreferences: Sendable, FeatureReducer {
case let .turnIntoDappDefAccountType(manifest):
state.destination = .reviewTransactionTurningAccountIntoDappDefType(.init(
transactionManifest: manifest,
nonce: engineToolkitClient.generateTXNonce(),
signTransactionPurpose: .internalManifest(.debugModifyAccount),
message: nil
))
Expand Down
2 changes: 2 additions & 0 deletions Sources/Features/CreateAuthKey/CreateAuthKey.swift
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,7 @@ public struct CreateAuthKey: Sendable, FeatureReducer {
@Dependency(\.accountsClient) var accountsClient
@Dependency(\.personasClient) var personasClient
@Dependency(\.factorSourcesClient) var factorSourcesClient
@Dependency(\.engineToolkitClient) var engineToolkitClient

public init() {}

Expand Down Expand Up @@ -164,6 +165,7 @@ public struct CreateAuthKey: Sendable, FeatureReducer {
case let .createdManifestForAuthKeyCreation(manifest, authenticationSigningFactorInstance):
state.step = .transactionReview(.init(
transactionManifest: manifest,
nonce: engineToolkitClient.generateTXNonce(),
signTransactionPurpose: .internalManifest(.uploadAuthKey),
message: nil
))
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import AccountsClient
import AuthorizedDappsClient
import Cryptography
import EngineToolkitClient
import FeaturePrelude
import GatewaysClient
import PersonasClient
Expand Down Expand Up @@ -181,6 +182,7 @@ struct DappInteractionFlow: Sendable, FeatureReducer {
@Dependency(\.personasClient) var personasClient
@Dependency(\.accountsClient) var accountsClient
@Dependency(\.authorizedDappsClient) var authorizedDappsClient
@Dependency(\.engineToolkitClient) var engineToolkitClient

func reduce(into state: inout State, viewAction: ViewAction) -> EffectTask<Action> {
switch viewAction {
Expand Down Expand Up @@ -770,8 +772,10 @@ extension DappInteractionFlow.Destinations.State {
}

case let .remote(.send(item)):
@Dependency(\.engineToolkitClient) var engineToolkitClient
self = .relayed(anyItem, with: .reviewTransaction(.init(
transactionManifest: item.transactionManifest,
nonce: engineToolkitClient.generateTXNonce(),
signTransactionPurpose: .manifestFromDapp,
message: item.message
)))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,7 @@ extension PrepareForSigning.State {

// MARK: - PrepareForSigning.View
extension PrepareForSigning {
public struct ViewState: Equatable {
// TODO: declare some properties
}
public struct ViewState: Equatable {}

@MainActor
public struct View: SwiftUI.View {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,16 +10,18 @@ public struct PrepareForSigning: Sendable, FeatureReducer {
public let feePayer: Profile.Network.Account
public let networkID: NetworkID
public let purpose: SigningPurpose

public let nonce: Nonce
public var compiledIntent: CompileTransactionIntentResponse? = nil
public let ephemeralNotaryPublicKey: Curve25519.Signing.PublicKey
public init(
nonce: Nonce,
manifest: TransactionManifest,
networkID: NetworkID,
feePayer: Profile.Network.Account,
purpose: SigningPurpose,
ephemeralNotaryPublicKey: Curve25519.Signing.PublicKey
) {
self.nonce = nonce
self.manifest = manifest
self.networkID = networkID
self.feePayer = feePayer
Expand Down Expand Up @@ -98,6 +100,7 @@ public struct PrepareForSigning: Sendable, FeatureReducer {
try await transactionClient.buildTransactionIntent(.init(
networkID: state.networkID,
manifest: state.manifest,
nonce: state.nonce,
ephemeralNotaryPublicKey: state.ephemeralNotaryPublicKey
))
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@ extension TransactionReview.State {
viewControlState: viewControlState,
showDottedLine: (withdrawals != nil || message != nil) && deposits != nil,
rawTransaction: displayMode.rawTransaction,
showApproveButton: transactionWithLockFee != nil
showApproveButton: transactionWithLockFee != nil,
canApproveTX: canApproveTX
)
}

Expand All @@ -46,6 +47,7 @@ extension TransactionReview {
let showDottedLine: Bool
let rawTransaction: String?
let showApproveButton: Bool
let canApproveTX: Bool
}

@MainActor
Expand Down Expand Up @@ -119,6 +121,7 @@ extension TransactionReview {
Button(L10n.TransactionReview.approveButtonTitle, asset: AssetResource.lock) {
viewStore.send(.approveTapped)
}
.controlState(viewStore.canApproveTX ? .enabled : .disabled)
.buttonStyle(.primaryRectangular)
.padding(.bottom, .medium1)
}
Expand Down
24 changes: 20 additions & 4 deletions Sources/Features/TransactionReviewFeature/TransactionReview.swift
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ public struct TransactionReview: Sendable, FeatureReducer {
public struct State: Sendable, Hashable {
public var displayMode: DisplayMode = .review

public let nonce: Nonce
public let transactionManifest: TransactionManifest
public let message: String?
public let signTransactionPurpose: SigningPurpose.SignTransactionPurpose
Expand All @@ -30,18 +31,21 @@ public struct TransactionReview: Sendable, FeatureReducer {
public var proofs: TransactionReviewProofs.State? = nil
public var networkFee: TransactionReviewNetworkFee.State? = nil
public let ephemeralNotaryPrivateKey: Curve25519.Signing.PrivateKey
public var canApproveTX: Bool = true

@PresentationState
public var destination: Destinations.State? = nil

public init(
transactionManifest: TransactionManifest,
nonce: Nonce,
signTransactionPurpose: SigningPurpose.SignTransactionPurpose,
message: String?,
feeToAdd: BigDecimal = 10, // fix me use estimate from `analyze`
ephemeralNotaryPrivateKey: Curve25519.Signing.PrivateKey = .init(),
customizeGuarantees: TransactionReviewGuarantees.State? = nil
) {
self.nonce = nonce
self.transactionManifest = transactionManifest
self.signTransactionPurpose = signTransactionPurpose
self.message = message
Expand Down Expand Up @@ -159,17 +163,17 @@ public struct TransactionReview: Sendable, FeatureReducer {
.ifLet(\.$destination, action: /Action.child .. ChildAction.destination) {
Destinations()
}
.debug()
}

public func reduce(into state: inout State, viewAction: ViewAction) -> EffectTask<Action> {
switch viewAction {
case .appeared:
let manifest = state.transactionManifest
return .task { [feeToAdd = state.fee] in
return .task { [feeToAdd = state.fee, nonce = state.nonce] in
await .internal(.previewLoaded(TaskResult {
try await transactionClient.getTransactionReview(.init(
manifestToSign: manifest,
nonce: nonce,
feeToAdd: feeToAdd
))
}))
Expand Down Expand Up @@ -198,6 +202,7 @@ public struct TransactionReview: Sendable, FeatureReducer {

case .approveTapped:
guard let transactionWithLockFee = state.transactionWithLockFee else { return .none }
state.canApproveTX = false

let guarantees = state.allGuarantees

Expand Down Expand Up @@ -255,9 +260,11 @@ public struct TransactionReview: Sendable, FeatureReducer {
}

case .destination(.presented(.prepareForSigning(.delegate(.failedToBuildTX)))):
state.canApproveTX = true
return .none

case .destination(.presented(.prepareForSigning(.delegate(.failedToLoadSigners)))):
state.canApproveTX = true
return .none

case let .destination(.presented(.prepareForSigning(.delegate(.done(compiledIntent, signingFactors))))):
Expand Down Expand Up @@ -293,28 +300,36 @@ public struct TransactionReview: Sendable, FeatureReducer {
case .destination(.presented(.signing(.delegate(.failedToSign)))):
loggerGlobal.error("Failed sign tx")
state.destination = nil
state.canApproveTX = true
return .none

case let .destination(.presented(.signing(.delegate(.finishedSigning(.signTransaction(notarizedTX, origin: _)))))):
state.destination = .submitting(.init(notarizedTX: notarizedTX))
return .none

case .destination(.presented(.signing(.delegate(.finishedSigning(.signAuth(_)))))):
state.canApproveTX = true
assertionFailure("Did not expect to have sign auth data...")
return .none

case let .destination(.presented(.submitting(.delegate(.submittedButNotCompleted(txID))))):
return .send(.delegate(.signedTXAndSubmittedToGateway(txID)))

case
.destination(.presented(.submitting(.delegate(.failedToSubmit)))),
.destination(.presented(.submitting(.delegate(.failedToReceiveStatusUpdate)))):
.destination(.presented(.submitting(.delegate(.failedToSubmit)))):
state.destination = nil
state.canApproveTX = true
loggerGlobal.error("Failed to submit tx")
return .none

case .destination(.presented(.submitting(.delegate(.failedToReceiveStatusUpdate)))):
state.destination = nil
loggerGlobal.error("Failed to receive status update")
return .none

case .destination(.presented(.submitting(.delegate(.submittedTransactionFailed)))):
state.destination = nil
state.canApproveTX = true
loggerGlobal.error("Submitted TX failed")
return .send(.delegate(.failed(.failedToSubmit)))

Expand Down Expand Up @@ -420,6 +435,7 @@ public struct TransactionReview: Sendable, FeatureReducer {
}

state.destination = .prepareForSigning(.init(
nonce: state.nonce,
manifest: manifest,
networkID: networkID,
feePayer: feePayerSelectionAmongstCandidates.selected.account,
Expand Down
1 change: 1 addition & 0 deletions Tests/Clients/FaucetClientTests/FaucetClientTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,7 @@ final class FaucetClientTests: TestCase {
)
)
}
$0.engineToolkitClient.generateTXNonce = { .init(0xDEAD) }
$0.engineToolkitClient.compileTransactionIntent = { _ in try .init(compiledIntentHex: "") }
$0.transactionClient.notarizeTransaction = { _ in NotarizeTransactionResponse(notarized: .init(compiledIntent: []), txID: .init("mocked_txid")) }
$0.submitTXClient.hasTXBeenCommittedSuccessfully = { _ in }
Expand Down

0 comments on commit 4fd9b9a

Please sign in to comment.