-
Notifications
You must be signed in to change notification settings - Fork 33
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
Expose APIs for working with transaction proposals #1382
Merged
Merged
Changes from all commits
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
8145248
Expose APIs for working with transaction proposals
str4d dd41e65
Deprecate `Synchronizer.sendToAddress` and `Synchronizer.shieldFunds`
str4d 136a311
Documentation fixes
str4d 84ac625
Adjust `Synchronizer.proposeShielding` API
str4d bb1a05e
Enable ZIP 317 fees
LukasKorba f617f17
Add testOnlyFakeProposal for testing purposes outside SDK
LukasKorba e9177a2
Migrate to in-progress version of FFI backend 0.6.0
str4d 9b2be55
Add `Proposal.transactionCount`
str4d 23fd069
Migrate to FFI 0.6.0 release
str4d 129ac43
Add missing changes to protocols for `Synchronizer` wrappers
str4d File filter
Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,45 @@ | ||
// | ||
// Proposal.swift | ||
// | ||
// | ||
// Created by Jack Grigg on 20/02/2024. | ||
// | ||
|
||
import Foundation | ||
|
||
/// A data structure that describes a series of transactions to be created. | ||
public struct Proposal: Equatable { | ||
let inner: FfiProposal | ||
|
||
/// Returns the number of transactions that this proposal will create. | ||
/// | ||
/// This is equal to the number of `TransactionSubmitResult`s that will be returned | ||
/// from `Synchronizer.createProposedTransactions`. | ||
/// | ||
/// Proposals always create at least one transaction. | ||
public func transactionCount() -> Int { | ||
inner.steps.count | ||
} | ||
|
||
/// Returns the total fee to be paid across all proposed transactions, in zatoshis. | ||
public func totalFeeRequired() -> Zatoshi { | ||
inner.steps.reduce(Zatoshi.zero) { acc, step in | ||
acc + Zatoshi(Int64(step.balance.feeRequired)) | ||
} | ||
} | ||
} | ||
|
||
public extension Proposal { | ||
/// IMPORTANT: This function is for testing purposes only. It produces fake invalid | ||
/// data that can be used to check UI elements, but will always produce an error when | ||
/// passed to `Synchronizer.createProposedTransactions`. It should never be called in | ||
/// production code. | ||
static func testOnlyFakeProposal(totalFee: UInt64) -> Self { | ||
var ffiProposal = FfiProposal() | ||
var balance = FfiTransactionBalance() | ||
|
||
balance.feeRequired = totalFee | ||
|
||
return Self(inner: ffiProposal) | ||
} | ||
} |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would advise not to expose this publicly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you have a suggestion for how to avoid exposing this publicly, I'm all ears. The constraint here is Zashi iOS using TCA, which needs to be able to construct mock data to be returned from its mock implementation of the
Synchronizer
protocol for use in e.g. preview of UIs (so we can't just return something that would be an error case).There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NOTE: I don't know which things you've tried and which ones you haven't, but there are a few things I can think from the top of my head. I promise I'll look this deeper shortly. I might be still lacking context and this could be somewhat inaccurate or insufficient.
A. "Uncle bob's Clean code" approach
Treat the whole SDK as an external dependency under the principle of Bob Martin's clean code architecture. Look at the problem as you were Unstoppable or EDGE and you don't control the dependency. Wrap around it and have a shim that you Do control and use that instead.
B. Pointfree.co / TCA approach.
Make proposal on the SDK Protocol. The SDK returns an implementation of it. Then use a protocol witness or a mock implementation on Zashi to provide the mocked version that you need.
A and B
Make a protocol on Zashi for what the app expects of a proposal. This propocol contains a throwing
toProposal()
function.Make an Adapter of the SDK's Proposal type that conforms to Zashi's.
Make a Mock implementation for testing on Zashi that throws whenever
toProposal()
is invoked. If by mistake this ever happens and a mocked proposal is sent in production you can see the error and flag it as critical in Crash reporting platform.Or better, the testing code is never available in outside of the testing target.