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

Tests for tracking known transactions #148

Merged
merged 7 commits into from
Apr 8, 2019
Merged

Conversation

rvl
Copy link
Contributor

@rvl rvl commented Apr 4, 2019

Relates to issue #90

Overview

This is a start for tests of the transaction tracking part of applyBlock.

prop_applyBlockTxHistoryIncoming :: WalletState -> Property
prop_applyBlockTxHistoryIncoming s = conjoin (map prop wallets)
where
wallets = scanl (flip applyBlock) (initWallet s) blockchain
Copy link
Member

Choose a reason for hiding this comment

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

Is there a particular reason to check all intermediate state and not only the last one 🤔 ?

inTxs = filter isIncoming $ Set.toList $ getTxHistory wallet
isIncoming (_, m) = direction m == Incoming
txOuts = map (Set.fromList . map address . outputs . fst) inTxs
overlaps a b = not (Set.disjoint a b)
Copy link
Member

Choose a reason for hiding this comment

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

It'll be nice to add a coverage checks that makes sure that addrs and ourAddresses aren't just empty all the time.

it "Incoming transactions have output addresses that belong to the wallet"
(property prop_applyBlockTxHistoryIncoming)
it "Any transction involving our addresses must be incoming"
(property prop_applyBlockTxHistoryOurs)
Copy link
Member

Choose a reason for hiding this comment

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

I don't get this second property 🤔 ? What about outgoing transactions with change?

rvl and others added 7 commits April 8, 2019 11:35
@KtorZ KtorZ force-pushed the rvl/90/transaction-properties branch from 29cd470 to 6aca526 Compare April 8, 2019 12:39
Copy link
Member

@KtorZ KtorZ left a comment

Choose a reason for hiding this comment

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

reviewed + extended test data to cover more cases / branches from the model.

@KtorZ KtorZ merged commit 1cd66fc into master Apr 8, 2019
@KtorZ KtorZ deleted the rvl/90/transaction-properties branch April 8, 2019 13:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants