[#781] This fixes test `testMaxAmountMinusOneSend` by creating two se… (#1034)

* [#781] This fixes test `testMaxAmountMinusOneSend` by creating two separate
tests:
  - testMaxAmountMinusOneSendFails
  - testMaxAmountSend

Also includes new functionality that tracks sent transactions so
that users can be notified specifically when they are mined and uses "idea B" of
issue #1033.

closes #1033
closes #781

* Fix tests
This commit is contained in:
Francisco Gindre 2023-05-10 17:13:29 -03:00 committed by GitHub
parent cf53edb3b8
commit f98692c0ea
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 248 additions and 133 deletions

View File

@ -58,16 +58,30 @@ public enum CompactBlockProgress {
}
public struct EnhancementProgress: Equatable {
/// total transactions that were detected in the `range`
public let totalTransactions: Int
/// enhanced transactions so far
public let enhancedTransactions: Int
/// last found transaction
public let lastFoundTransaction: ZcashTransaction.Overview?
/// block range that's being enhanced
public let range: CompactBlockRange
/// whether this transaction can be considered `newly mined` and not part of the
/// wallet catching up to stale and uneventful blocks.
public let newlyMined: Bool
public init(totalTransactions: Int, enhancedTransactions: Int, lastFoundTransaction: ZcashTransaction.Overview?, range: CompactBlockRange) {
public init(
totalTransactions: Int,
enhancedTransactions: Int,
lastFoundTransaction: ZcashTransaction.Overview?,
range: CompactBlockRange,
newlyMined: Bool
) {
self.totalTransactions = totalTransactions
self.enhancedTransactions = enhancedTransactions
self.lastFoundTransaction = lastFoundTransaction
self.range = range
self.newlyMined = newlyMined
}
public var progress: Float {
@ -75,7 +89,7 @@ public struct EnhancementProgress: Equatable {
}
public static var zero: EnhancementProgress {
EnhancementProgress(totalTransactions: 0, enhancedTransactions: 0, lastFoundTransaction: nil, range: 0...0)
EnhancementProgress(totalTransactions: 0, enhancedTransactions: 0, lastFoundTransaction: nil, range: 0...0, newlyMined: false)
}
public static func == (lhs: EnhancementProgress, rhs: EnhancementProgress) -> Bool {
@ -99,6 +113,9 @@ actor CompactBlockProcessor {
/// Event sent when the CompactBlockProcessor has finished syncing the blockchain to latest height
case finished (_ lastScannedHeight: BlockHeight, _ foundBlocks: Bool)
/// Event sent when the CompactBlockProcessor found a newly mined transaction
case minedTransaction(ZcashTransaction.Overview)
/// Event sent when the CompactBlockProcessor enhanced a bunch of transactions in some range.
case foundTransactions ([ZcashTransaction.Overview], CompactBlockRange)
@ -672,10 +689,19 @@ actor CompactBlockProcessor {
anyActionExecuted = true
logger.debug("Enhancing with range: \(range.lowerBound)...\(range.upperBound)")
await updateState(.enhancing)
let transactions = try await blockEnhancer.enhance(at: range) { [weak self] progress in
await self?.notifyProgress(.enhance(progress))
if let transactions = try await blockEnhancer.enhance(
at: range,
didEnhance: { [weak self] progress in
await self?.notifyProgress(.enhance(progress))
if
let foundTx = progress.lastFoundTransaction,
progress.newlyMined {
await self?.notifyMinedTransaction(foundTx)
}
}
) {
await notifyTransactions(transactions, in: range)
}
await notifyTransactions(transactions, in: range)
}
if let range = ranges.fetchUTXORange {
@ -834,6 +860,11 @@ actor CompactBlockProcessor {
return lowerBound...upperBound
}
func notifyMinedTransaction(_ tx: ZcashTransaction.Overview) async {
logger.debug("notify mined transaction: \(tx)")
await send(event: .minedTransaction(tx))
}
func notifyProgress(_ progress: CompactBlockProgress) async {
logger.debug("progress: \(progress)")
await send(event: .progressUpdated(progress))

View File

@ -8,7 +8,7 @@
import Foundation
protocol BlockEnhancer {
func enhance(at range: CompactBlockRange, didEnhance: (EnhancementProgress) async -> Void) async throws -> [ZcashTransaction.Overview]
func enhance(at range: CompactBlockRange, didEnhance: (EnhancementProgress) async -> Void) async throws -> [ZcashTransaction.Overview]?
}
struct BlockEnhancerImpl {
@ -38,7 +38,7 @@ struct BlockEnhancerImpl {
}
extension BlockEnhancerImpl: BlockEnhancer {
func enhance(at range: CompactBlockRange, didEnhance: (EnhancementProgress) async -> Void) async throws -> [ZcashTransaction.Overview] {
func enhance(at range: CompactBlockRange, didEnhance: (EnhancementProgress) async -> Void) async throws -> [ZcashTransaction.Overview]? {
try Task.checkCancellation()
logger.debug("Started Enhancing range: \(range)")
@ -54,9 +54,16 @@ extension BlockEnhancerImpl: BlockEnhancer {
guard !transactions.isEmpty else {
await internalSyncProgress.set(range.upperBound, .latestEnhancedHeight)
logger.debug("no transactions detected on range: \(range.lowerBound)...\(range.upperBound)")
return []
return nil
}
let chainTipHeight = try await blockDownloaderService.latestBlockHeight()
let newlyMinedLowerBound = chainTipHeight - ZcashSDK.expiryOffset
let newlyMinedRange = newlyMinedLowerBound...chainTipHeight
for index in 0 ..< transactions.count {
let transaction = transactions[index]
var retry = true
@ -66,12 +73,15 @@ extension BlockEnhancerImpl: BlockEnhancer {
do {
let confirmedTx = try await enhance(transaction: transaction)
retry = false
let progress = EnhancementProgress(
totalTransactions: transactions.count,
enhancedTransactions: index + 1,
lastFoundTransaction: confirmedTx,
range: range
range: range,
newlyMined: confirmedTx.isSentTransaction && newlyMinedRange.contains(confirmedTx.minedHeight ?? 0)
)
await didEnhance(progress)
if let minedHeight = confirmedTx.minedHeight {
@ -109,6 +119,6 @@ extension BlockEnhancerImpl: BlockEnhancer {
logger.debug("Warning: compactBlockEnhancement on range \(range) cancelled")
}
return (try? await transactionRepository.find(in: range, limit: Int.max, kind: .all)) ?? []
return (try? await transactionRepository.find(in: range, limit: Int.max, kind: .all))
}
}

View File

@ -84,8 +84,9 @@ public struct SynchronizerState: Equatable {
}
public enum SynchronizerEvent {
// Sent when the synchronizer finds a pendingTransaction that hast been newly mined.
// Sent when the synchronizer finds a pendingTransaction that has been newly mined.
case minedTransaction(ZcashTransaction.Overview)
// Sent when the synchronizer finds a mined transaction
case foundTransactions(_ transactions: [ZcashTransaction.Overview], _ inRange: CompactBlockRange)
// Sent when the synchronizer fetched utxos from lightwalletd attempted to store them.

View File

@ -230,6 +230,9 @@ public class SDKSynchronizer: Synchronizer {
case .stopped:
await self?.updateStatus(.stopped)
case .minedTransaction(let transaction):
self?.notifyMinedTransaction(transaction)
}
}

View File

@ -193,18 +193,162 @@ class BalanceTests: ZcashTestCase {
XCTAssertEqual(expectedBalance, .zero)
XCTAssertEqual(expectedVerifiedBalance, .zero)
}
/**
verify that when sending the maximum amount minus one zatoshi, the transactions are broadcasted properly
*/
func testMaxAmountSend() async throws {
let notificationHandler = SDKSynchonizerListener()
let foundTransactionsExpectation = XCTestExpectation(description: "found transactions expectation")
let transactionMinedExpectation = XCTestExpectation(description: "transaction mined expectation")
// 0 subscribe to updated transactions events
notificationHandler.subscribeToSynchronizer(coordinator.synchronizer)
// 1 sync and get spendable funds
try FakeChainBuilder.buildChain(darksideWallet: coordinator.service, branchID: branchID, chainName: chainName)
try coordinator.applyStaged(blockheight: defaultLatestHeight + 10)
sleep(1)
let firstSyncExpectation = XCTestExpectation(description: "first sync expectation")
do {
try await coordinator.sync(
completion: { _ in
firstSyncExpectation.fulfill()
},
error: self.handleError
)
} catch {
handleError(error)
}
await fulfillment(of: [firstSyncExpectation], timeout: 12)
// 2 check that there are no unconfirmed funds
let verifiedBalance: Zatoshi = try await coordinator.synchronizer.getShieldedVerifiedBalance()
let totalBalance: Zatoshi = try await coordinator.synchronizer.getShieldedBalance()
XCTAssertTrue(verifiedBalance > network.constants.defaultFee(for: defaultLatestHeight))
XCTAssertEqual(verifiedBalance, totalBalance)
let maxBalanceMinusOne = verifiedBalance - Zatoshi(1000)
// 3 create a transaction for the max amount possible
// 4 send the transaction
let spendingKey = coordinator.spendingKey
var pendingTx: ZcashTransaction.Overview?
do {
let transaction = try await coordinator.synchronizer.sendToAddress(
spendingKey: spendingKey,
zatoshi: maxBalanceMinusOne,
toAddress: try Recipient(Environment.testRecipientAddress, network: self.network.networkType),
memo: try Memo(string: "\(self.description) \(Date().description)")
)
pendingTx = transaction
self.sentTransactionExpectation.fulfill()
} catch {
XCTFail("sendToAddress failed: \(error)")
}
await fulfillment(of: [sentTransactionExpectation], timeout: 20)
guard let pendingTx else {
XCTFail("transaction creation failed")
return
}
notificationHandler.synchronizerMinedTransaction = { transaction in
XCTAssertNotNil(transaction.rawID)
XCTAssertNotNil(pendingTx.rawID)
XCTAssertEqual(transaction.rawID, pendingTx.rawID)
transactionMinedExpectation.fulfill()
}
// 5 apply to height
// 6 mine the block
guard let rawTx = try coordinator.getIncomingTransactions()?.first else {
XCTFail("no incoming transaction after")
return
}
let latestHeight = try await coordinator.latestHeight()
let sentTxHeight = latestHeight + 1
notificationHandler.transactionsFound = { txs in
let foundTx = txs.first(where: { $0.rawID == pendingTx.rawID })
XCTAssertNotNil(foundTx)
XCTAssertEqual(foundTx?.minedHeight, sentTxHeight)
foundTransactionsExpectation.fulfill()
}
try coordinator.stageBlockCreate(height: sentTxHeight, count: 100)
sleep(1)
try coordinator.stageTransaction(rawTx, at: sentTxHeight)
try coordinator.applyStaged(blockheight: sentTxHeight)
sleep(2) // add enhance breakpoint here
let mineExpectation = XCTestExpectation(description: "mineTxExpectation")
do {
try await coordinator.sync(
completion: { synchronizer in
let pendingEntity = try await synchronizer.allPendingTransactions().first(where: { $0.rawID == pendingTx.rawID })
XCTAssertNotNil(pendingEntity, "pending transaction should have been mined by now")
XCTAssertNotNil(pendingEntity?.minedHeight)
XCTAssertEqual(pendingEntity?.minedHeight, sentTxHeight)
mineExpectation.fulfill()
},
error: self.handleError
)
} catch {
handleError(error)
}
await fulfillment(of: [mineExpectation, transactionMinedExpectation, foundTransactionsExpectation], timeout: 5)
// 7 advance to confirmation
let advanceToConfirmationHeight = sentTxHeight + 10
try coordinator.applyStaged(blockheight: advanceToConfirmationHeight)
sleep(2)
let confirmExpectation = XCTestExpectation(description: "confirm expectation")
notificationHandler.transactionsFound = { txs in
XCTFail("We shouldn't find any transactions at this point but found \(txs)")
}
notificationHandler.synchronizerMinedTransaction = { transaction in
XCTFail("We shouldn't find any mined transactions at this point but found \(transaction)")
}
do {
try await coordinator.sync(
completion: { _ in
confirmExpectation.fulfill()
},
error: self.handleError
)
} catch {
handleError(error)
}
await fulfillment(of: [confirmExpectation], timeout: 5)
let confirmedPending = try await coordinator.synchronizer
.allPendingTransactions()
.first(where: { $0.rawID == pendingTx.rawID })
XCTAssertNil(confirmedPending, "pending, now confirmed transaction found")
let expectedVerifiedBalance = try await coordinator.synchronizer.getShieldedVerifiedBalance()
let expectedBalance = try await coordinator.synchronizer.getShieldedBalance()
XCTAssertEqual(expectedBalance, .zero)
XCTAssertEqual(expectedVerifiedBalance, .zero)
}
/**
verify that when sending the maximum amount minus one zatoshi, the transactions are broadcasted properly
*/
// FIXME [#781]: Fix test
func disabled_testMaxAmountMinusOneSend() async throws {
let notificationHandler = SDKSynchonizerListener()
let foundTransactionsExpectation = XCTestExpectation(description: "found transactions expectation")
let transactionMinedExpectation = XCTestExpectation(description: "transaction mined expectation")
// 0 subscribe to updated transactions events
notificationHandler.subscribeToSynchronizer(coordinator.synchronizer)
func testMaxAmountMinusOneSendFails() async throws {
// 1 sync and get spendable funds
try FakeChainBuilder.buildChain(darksideWallet: coordinator.service, branchID: branchID, chainName: chainName)
@ -232,117 +376,32 @@ class BalanceTests: ZcashTestCase {
XCTAssertTrue(verifiedBalance > network.constants.defaultFee(for: defaultLatestHeight))
XCTAssertEqual(verifiedBalance, totalBalance)
let maxBalanceMinusOne = verifiedBalance - network.constants.defaultFee(for: defaultLatestHeight) - Zatoshi(1)
let maxBalanceMinusOne = verifiedBalance - Zatoshi(1000) - Zatoshi(1)
// 3 create a transaction for the max amount possible
// 4 send the transaction
let spendingKey = coordinator.spendingKey
var pendingTx: ZcashTransaction.Overview?
do {
let transaction = try await coordinator.synchronizer.sendToAddress(
_ = try await coordinator.synchronizer.sendToAddress(
spendingKey: spendingKey,
zatoshi: maxBalanceMinusOne,
toAddress: try Recipient(Environment.testRecipientAddress, network: self.network.networkType),
memo: try Memo(string: "\(self.description) \(Date().description)")
)
pendingTx = transaction
self.sentTransactionExpectation.fulfill()
} catch {
XCTFail("sendToAddress failed: \(error)")
}
guard
let zcashError = error as? ZcashError,
case let ZcashError.rustCreateToAddress(message) = zcashError
else {
XCTFail("Expected ZcashError and found \(error)")
return
}
await fulfillment(of: [sentTransactionExpectation], timeout: 20)
guard let pendingTx else {
XCTFail("transaction creation failed")
XCTAssertEqual(message, "Error while sending funds: Insufficient balance (have 200000, need 200999 including fee)")
return
}
notificationHandler.synchronizerMinedTransaction = { transaction in
XCTAssertNotNil(transaction.rawID)
XCTAssertNotNil(pendingTx.rawID)
XCTAssertEqual(transaction.rawID, pendingTx.rawID)
transactionMinedExpectation.fulfill()
}
// 5 apply to height
// 6 mine the block
guard let rawTx = try coordinator.getIncomingTransactions()?.first else {
XCTFail("no incoming transaction after")
return
}
let latestHeight = try await coordinator.latestHeight()
let sentTxHeight = latestHeight + 1
notificationHandler.transactionsFound = { txs in
let foundTx = txs.first(where: { $0.rawID == pendingTx.rawID })
XCTAssertNotNil(foundTx)
XCTAssertEqual(foundTx?.minedHeight, sentTxHeight)
foundTransactionsExpectation.fulfill()
}
try coordinator.stageBlockCreate(height: sentTxHeight, count: 100)
sleep(1)
try coordinator.stageTransaction(rawTx, at: sentTxHeight)
try coordinator.applyStaged(blockheight: sentTxHeight)
sleep(2) // add enhance breakpoint here
let mineExpectation = XCTestExpectation(description: "mineTxExpectation")
do {
try await coordinator.sync(
completion: { synchronizer in
let pendingEntity = try await synchronizer.allPendingTransactions().first(where: { $0.rawID == pendingTx.rawID })
XCTAssertNotNil(pendingEntity, "pending transaction should have been mined by now")
XCTAssertNotNil(pendingEntity?.minedHeight)
XCTAssertEqual(pendingEntity?.minedHeight, sentTxHeight)
mineExpectation.fulfill()
},
error: self.handleError
)
} catch {
handleError(error)
}
await fulfillment(of: [mineExpectation, transactionMinedExpectation, foundTransactionsExpectation], timeout: 5)
// 7 advance to confirmation
let advanceToConfirmationHeight = sentTxHeight + 10
try coordinator.applyStaged(blockheight: advanceToConfirmationHeight)
sleep(2)
let confirmExpectation = XCTestExpectation(description: "confirm expectation")
notificationHandler.transactionsFound = { txs in
XCTFail("We shouldn't find any transactions at this point but found \(txs)")
}
notificationHandler.synchronizerMinedTransaction = { transaction in
XCTFail("We shouldn't find any mined transactions at this point but found \(transaction)")
}
do {
try await coordinator.sync(
completion: { _ in
confirmExpectation.fulfill()
},
error: self.handleError
)
} catch {
handleError(error)
}
await fulfillment(of: [confirmExpectation], timeout: 5)
let confirmedPending = try await coordinator.synchronizer
.allPendingTransactions()
.first(where: { $0.rawID == pendingTx.rawID })
XCTAssertNil(confirmedPending, "pending, now confirmed transaction found")
let expectedVerifiedBalance = try await coordinator.synchronizer.getShieldedVerifiedBalance()
let expectedBalance = try await coordinator.synchronizer.getShieldedBalance()
XCTAssertEqual(expectedBalance, Zatoshi(1))
XCTAssertEqual(expectedVerifiedBalance, Zatoshi(1))
XCTFail("This should have failed with Insufficient funds error")
}
/**

View File

@ -213,7 +213,7 @@ class SynchronizerDarksideTests: ZcashTestCase {
syncSessionID: uuids[0],
shieldedBalance: WalletBalance(verified: Zatoshi(100000), total: Zatoshi(200000)),
transparentBalance: .zero,
internalSyncStatus: .enhancing(EnhancementProgress(totalTransactions: 0, enhancedTransactions: 0, lastFoundTransaction: nil, range: 0...0)),
internalSyncStatus: .enhancing(EnhancementProgress(totalTransactions: 0, enhancedTransactions: 0, lastFoundTransaction: nil, range: 0...0, newlyMined: false)),
latestScannedHeight: 663189,
latestBlockHeight: 663189,
latestScannedTime: 1
@ -243,7 +243,8 @@ class SynchronizerDarksideTests: ZcashTestCase {
value: Zatoshi(100000),
isExpiredUmined: false
),
range: 663150...663189
range: 663150...663189,
newlyMined: true
)
),
latestScannedHeight: 663189,
@ -275,7 +276,8 @@ class SynchronizerDarksideTests: ZcashTestCase {
value: Zatoshi(100000),
isExpiredUmined: false
),
range: 663150...663189
range: 663150...663189,
newlyMined: true
)
),
latestScannedHeight: 663189,
@ -374,7 +376,7 @@ class SynchronizerDarksideTests: ZcashTestCase {
shieldedBalance: WalletBalance(verified: Zatoshi(100000), total: Zatoshi(200000)),
transparentBalance: WalletBalance(verified: Zatoshi(0), total: Zatoshi(0)),
internalSyncStatus: .enhancing(
EnhancementProgress(totalTransactions: 0, enhancedTransactions: 0, lastFoundTransaction: nil, range: 0...0)
EnhancementProgress(totalTransactions: 0, enhancedTransactions: 0, lastFoundTransaction: nil, range: 0...0, newlyMined: false)
),
latestScannedHeight: 663189,
latestBlockHeight: 663189,
@ -405,7 +407,8 @@ class SynchronizerDarksideTests: ZcashTestCase {
value: Zatoshi(100000),
isExpiredUmined: false
),
range: 663150...663189
range: 663150...663189,
newlyMined: true
)
),
latestScannedHeight: 663189,
@ -437,7 +440,8 @@ class SynchronizerDarksideTests: ZcashTestCase {
value: Zatoshi(100000),
isExpiredUmined: false
),
range: 663150...663189
range: 663150...663189,
newlyMined: true
)
),
latestScannedHeight: 663189,
@ -512,7 +516,7 @@ class SynchronizerDarksideTests: ZcashTestCase {
shieldedBalance: WalletBalance(verified: Zatoshi(200000), total: Zatoshi(200000)),
transparentBalance: WalletBalance(verified: Zatoshi(0), total: Zatoshi(0)),
internalSyncStatus: .enhancing(
EnhancementProgress(totalTransactions: 0, enhancedTransactions: 0, lastFoundTransaction: nil, range: 0...0)
EnhancementProgress(totalTransactions: 0, enhancedTransactions: 0, lastFoundTransaction: nil, range: 0...0, newlyMined: true)
),
latestScannedHeight: 663200,
latestBlockHeight: 663200,
@ -615,12 +619,6 @@ class SynchronizerDarksideTests: ZcashTestCase {
}
}
extension Zatoshi: CustomDebugStringConvertible {
public var debugDescription: String {
"Zatoshi(\(self.amount))"
}
}
extension UUID {
static let deadbeef = UUID(uuidString: "DEADBEEF-BEEF-FAFA-BEEF-FAFAFAFAFAFA")!
static let beefbeef = UUID(uuidString: "BEEFBEEF-BEEF-DEAD-BEEF-BEEFEBEEFEBE")!

View File

@ -476,7 +476,8 @@ class SynchronizerOfflineTests: ZcashTestCase {
totalTransactions: 100,
enhancedTransactions: 0,
lastFoundTransaction: nil,
range: CompactBlockRange(uncheckedBounds: (0, 100))
range: CompactBlockRange(uncheckedBounds: (0, 100)),
newlyMined: false
)
)
)
@ -494,7 +495,8 @@ class SynchronizerOfflineTests: ZcashTestCase {
totalTransactions: 100,
enhancedTransactions: 50,
lastFoundTransaction: nil,
range: CompactBlockRange(uncheckedBounds: (0, 100))
range: CompactBlockRange(uncheckedBounds: (0, 100)),
newlyMined: false
)
)
)
@ -512,7 +514,8 @@ class SynchronizerOfflineTests: ZcashTestCase {
totalTransactions: 100,
enhancedTransactions: 100,
lastFoundTransaction: nil,
range: CompactBlockRange(uncheckedBounds: (0, 100))
range: CompactBlockRange(uncheckedBounds: (0, 100)),
newlyMined: false
)
)
)

View File

@ -15,6 +15,7 @@ class CompactBlockProcessorEventHandler {
case failed
case finished
case foundTransactions
case minedTransaction
case handleReorg
case progressUpdated
case storedUTXOs
@ -60,6 +61,8 @@ extension CompactBlockProcessor.Event {
return .startedSyncing
case .stopped:
return .stopped
case .minedTransaction:
return .minedTransaction
}
}
}

View File

@ -151,3 +151,10 @@ extension ZcashRustBackend {
)
}
}
extension Zatoshi: CustomDebugStringConvertible {
public var debugDescription: String {
"Zatoshi(\(self.amount))"
}
}