Skip to content

Commit

Permalink
Merge #2335
Browse files Browse the repository at this point in the history
2335: Move selected primitive types to dedicated modules (Part 2). r=jonathanknowles a=jonathanknowles

# Issue Number

Preparation for [ADP-347](https://jira.iohk.io/browse/ADP-347) (_Wallet should support UTxOs containing multiple token types_).

# Overview

This PR cleans up a few loose ends left behind by PR #2329:

- Renames `ChimericAccount` to `RewardAccount`.
  (see [related discussion](#2329 (review)))
- Adds a module comment to `Primitive.Types.Tx`.
  (see [related discussion](#2329 (comment))) 
- Clarifies the module comment for `Primitive.Types.Address`.
  (see [related discussion](#2329 (comment)))

It also moves the following UTxO-related types (and functions) to a new module:

| Type | Destination Module |
| -- | -- |
| `UTxO` | `Cardano.Wallet.Primitive.Types.UTxO` |
| `UTxOStatistics` | `Cardano.Wallet.Primitive.Types.UTxO` |
| `Dom` | `Cardano.Wallet.Primitive.Types.UTxO` |
| `HistogramBar` | `Cardano.Wallet.Primitive.Types.UTxO` |

# Motivation

The primary motivation for making these changes now, rather than later on, is to make development of multi-asset work more efficient.

So this PR limits itself to types that are likely to be affected by the MA work, rather than breaking up the entirety of `Primitive.Types`.

# Compilation Time Reductions

Here's a comparison of how long it takes to re-compile the whole of `cardano-wallet-core` (including tests) after making a tiny change to the `UTxO` type:

Before applying PR #2329 and PR #2335:
```
0m46.404s
0m45.124s
0m44.854s
```
After applying PR #2329 and PR #2335:
```
0m28.524s
0m28.364s
0m28.324s
```
That's approximately a 37% reduction in wall-clock time, with only slight changes to our module hierarchy.

**Machine used:**
```
vendor_id       : GenuineIntel
cpu family      : 6
model           : 85
model name      : Intel(R) Xeon(R) CPU
stepping        : 7
microcode       : 0x1
cpu MHz         : 2800.174
cache size      : 33792 KB
```

**Command:**
```
time stack build --ghc-options "-j +RTS -A256m -n4m -RTS" --fast --test --no-run-tests cardano-wallet-core
```

# Analysis

This PR:
* makes it possible for code to use the above types without having to incur a dependency on the `Cardano.Wallet.Primitive.Types` module, which (together with the modules that depend on it) is currently still large and quite slow to recompile.
* makes it possible to compile the above types in parallel, saving overall compilation time.
* makes module caching more effective: changes to one of the sub-modules will not necessarily invalidate the cache for the others, meaning that the compiler can often get away with compiling less code when changes are made.

# Comments

As before, this PR does not re-export the moved types from `Primitive.Types`, thereby forcing users of these types to import them directly from the module in which they're defined. This reduces the extent to which `Primitive.Types` is a compilation bottleneck.

This PR follows the advice presented in [Keeping Compilation Fast](https://www.parsonsmatt.org/2019/11/27/keeping_compilation_fast.html) by Matt Parsons.

Co-authored-by: Jonathan Knowles <jonathan.knowles@iohk.io>
  • Loading branch information
iohk-bors[bot] and jonathanknowles committed Nov 19, 2020
2 parents eabbddb + e93f811 commit 9c9d134
Show file tree
Hide file tree
Showing 50 changed files with 628 additions and 567 deletions.
12 changes: 7 additions & 5 deletions lib/core-integration/src/Test/Integration/Framework/DSL.hs
Expand Up @@ -244,19 +244,14 @@ import Cardano.Wallet.Primitive.Types
( ActiveSlotCoefficient (..)
, EpochLength (..)
, EpochNo
, HistogramBar (..)
, PoolId (..)
, PoolMetadataGCStatus (..)
, PoolMetadataSource
, Settings
, SlotLength (..)
, SlotNo (..)
, SortOrder (..)
, UTxO (..)
, UTxOStatistics (..)
, WalletId (..)
, computeUtxoStatistics
, log10
)
import Cardano.Wallet.Primitive.Types.Address
( Address (..) )
Expand All @@ -266,6 +261,13 @@ import Cardano.Wallet.Primitive.Types.Hash
( Hash (..) )
import Cardano.Wallet.Primitive.Types.Tx
( TxIn (..), TxOut (..), TxStatus (..) )
import Cardano.Wallet.Primitive.Types.UTxO
( HistogramBar (..)
, UTxO (..)
, UTxOStatistics (..)
, computeUtxoStatistics
, log10
)
import Control.Arrow
( second )
import Control.Concurrent
Expand Down
3 changes: 2 additions & 1 deletion lib/core/cardano-wallet-core.cabal
Expand Up @@ -161,10 +161,11 @@ library
Cardano.Wallet.Primitive.Model
Cardano.Wallet.Primitive.Types
Cardano.Wallet.Primitive.Types.Address
Cardano.Wallet.Primitive.Types.ChimericAccount
Cardano.Wallet.Primitive.Types.Coin
Cardano.Wallet.Primitive.Types.Hash
Cardano.Wallet.Primitive.Types.RewardAccount
Cardano.Wallet.Primitive.Types.Tx
Cardano.Wallet.Primitive.Types.UTxO
Cardano.Wallet.Registry
Cardano.Wallet.Transaction
Cardano.Wallet.Unsafe
Expand Down
61 changes: 30 additions & 31 deletions lib/core/src/Cardano/Wallet.hs
Expand Up @@ -80,8 +80,8 @@ module Cardano.Wallet
, rollbackBlocks
, checkWalletIntegrity
, readNextWithdrawal
, readChimericAccount
, someChimericAccount
, readRewardAccount
, someRewardAccount
, queryRewardBalance
, ErrWalletAlreadyExists (..)
, ErrNoSuchWallet (..)
Expand All @@ -90,7 +90,7 @@ module Cardano.Wallet
, ErrFetchRewards (..)
, ErrCheckWalletIntegrity (..)
, ErrWalletNotResponding (..)
, ErrReadChimericAccount (..)
, ErrReadRewardAccount (..)

-- ** Address
, createChangeAddress
Expand Down Expand Up @@ -231,7 +231,7 @@ import Cardano.Wallet.Primitive.AddressDerivation
, Passphrase
, PaymentAddress (..)
, SoftDerivation (..)
, ToChimericAccount (..)
, ToRewardAccount (..)
, WalletKey (..)
, checkPassphrase
, deriveRewardAccount
Expand Down Expand Up @@ -313,28 +313,24 @@ import Cardano.Wallet.Primitive.Types
, Range (..)
, Signature (..)
, SortOrder (..)
, UTxO (..)
, UTxOStatistics
, WalletDelegation (..)
, WalletDelegationStatus (..)
, WalletId (..)
, WalletMetadata (..)
, WalletName (..)
, WalletPassphraseInfo (..)
, computeUtxoStatistics
, distance
, dlgCertPoolId
, log10
, wholeRange
)
import Cardano.Wallet.Primitive.Types.Address
( Address (..), AddressState (..) )
import Cardano.Wallet.Primitive.Types.ChimericAccount
( ChimericAccount (..) )
import Cardano.Wallet.Primitive.Types.Coin
( Coin (..) )
import Cardano.Wallet.Primitive.Types.Hash
( Hash (..) )
import Cardano.Wallet.Primitive.Types.RewardAccount
( RewardAccount (..) )
import Cardano.Wallet.Primitive.Types.Tx
( Direction (..)
, SealedTx (..)
Expand All @@ -351,6 +347,8 @@ import Cardano.Wallet.Primitive.Types.Tx
, fromTransactionInfo
, withdrawals
)
import Cardano.Wallet.Primitive.Types.UTxO
( UTxO (..), UTxOStatistics, computeUtxoStatistics, log10 )
import Cardano.Wallet.Transaction
( DelegationAction (..)
, ErrDecodeSignedTx (..)
Expand Down Expand Up @@ -453,6 +451,7 @@ import qualified Cardano.Wallet.Primitive.CoinSelection.Random as CoinSelection
import qualified Cardano.Wallet.Primitive.Types as W
import qualified Cardano.Wallet.Primitive.Types.Coin as W
import qualified Cardano.Wallet.Primitive.Types.Tx as W
import qualified Cardano.Wallet.Primitive.Types.UTxO as W
import qualified Data.ByteArray as BA
import qualified Data.ByteString as BS
import qualified Data.List as L
Expand Down Expand Up @@ -594,7 +593,7 @@ createWallet
( HasGenesisData ctx
, HasDBLayer s k ctx
, IsOurs s Address
, IsOurs s ChimericAccount
, IsOurs s RewardAccount
)
=> ctx
-> WalletId
Expand Down Expand Up @@ -788,7 +787,7 @@ restoreWallet
, HasDBLayer s k ctx
, HasGenesisData ctx
, IsOurs s Address
, IsOurs s ChimericAccount
, IsOurs s RewardAccount
)
=> ctx
-> WalletId
Expand Down Expand Up @@ -841,7 +840,7 @@ restoreBlocks
, HasDBLayer s k ctx
, HasGenesisData ctx
, IsOurs s Address
, IsOurs s ChimericAccount
, IsOurs s RewardAccount
, HasNetworkLayer t ctx
)
=> ctx
Expand Down Expand Up @@ -1012,7 +1011,7 @@ readNextWithdrawal ctx wid (Quantity withdrawal) = db & \DBLayer{..} -> do
minFee policy =
fromIntegral . getFee . minimumFee tl policy Nothing Nothing

readChimericAccount
readRewardAccount
:: forall ctx s k (n :: NetworkDiscriminant) shelley.
( HasDBLayer s k ctx
, shelley ~ SeqState n ShelleyKey
Expand All @@ -1021,18 +1020,18 @@ readChimericAccount
)
=> ctx
-> WalletId
-> ExceptT ErrReadChimericAccount IO (ChimericAccount, NonEmpty DerivationIndex)
readChimericAccount ctx wid = db & \DBLayer{..} -> do
cp <- withExceptT ErrReadChimericAccountNoSuchWallet
-> ExceptT ErrReadRewardAccount IO (RewardAccount, NonEmpty DerivationIndex)
readRewardAccount ctx wid = db & \DBLayer{..} -> do
cp <- withExceptT ErrReadRewardAccountNoSuchWallet
$ mapExceptT atomically
$ withNoSuchWallet wid
$ readCheckpoint (PrimaryKey wid)
case testEquality (typeRep @s) (typeRep @shelley) of
Nothing ->
throwE ErrReadChimericAccountNotAShelleyWallet
throwE ErrReadRewardAccountNotAShelleyWallet
Just Refl -> do
let s = getState cp
let acct = toChimericAccount $ Seq.rewardAccountKey s
let acct = toRewardAccount $ Seq.rewardAccountKey s
let path = stakeDerivationPath $ Seq.derivationPrefix s
pure (acct, path)
where
Expand All @@ -1047,7 +1046,7 @@ queryRewardBalance
( HasNetworkLayer t ctx
)
=> ctx
-> ChimericAccount
-> RewardAccount
-> ExceptT ErrFetchRewards IO (Quantity "lovelace" Word64)
queryRewardBalance ctx acct = do
mapExceptT (fmap handleErr) $ getAccountBalance nw acct
Expand Down Expand Up @@ -1077,8 +1076,8 @@ manageRewardBalance _ ctx wid = db & \DBLayer{..} -> do
watchNodeTip $ \bh -> do
traceWith tr $ MsgRewardBalanceQuery bh
query <- runExceptT $ do
(acct, _) <- withExceptT ErrFetchRewardsReadChimericAccount $
readChimericAccount @ctx @s @k @n ctx wid
(acct, _) <- withExceptT ErrFetchRewardsReadRewardAccount $
readRewardAccount @ctx @s @k @n ctx wid
queryRewardBalance @ctx @t ctx acct
traceWith tr $ MsgRewardBalanceResult query
case query of
Expand Down Expand Up @@ -1612,7 +1611,7 @@ signPayment
( HasTransactionLayer t k ctx
, HasDBLayer s k ctx
, HasNetworkLayer t ctx
, IsOurs s ChimericAccount
, IsOurs s RewardAccount
, IsOwned s k
, GenChange s
)
Expand Down Expand Up @@ -1682,7 +1681,7 @@ signTx
( HasTransactionLayer t k ctx
, HasDBLayer s k ctx
, HasNetworkLayer t ctx
, IsOurs s ChimericAccount
, IsOurs s RewardAccount
, IsOwned s k
, HardDerivation k
, Bounded (Index (AddressIndexDerivationType k) 'AddressK)
Expand Down Expand Up @@ -1802,7 +1801,7 @@ signDelegation
, HasDBLayer s k ctx
, HasNetworkLayer t ctx
, IsOwned s k
, IsOurs s ChimericAccount
, IsOurs s RewardAccount
, GenChange s
, HardDerivation k
, AddressIndexDerivationType k ~ 'Soft
Expand Down Expand Up @@ -1870,7 +1869,7 @@ signDelegation ctx wid argGenChange pwd coinSel action = db & \DBLayer{..} -> do
-- FIXME: There's a logic duplication regarding the calculation of the transaction
-- amount between right here, and the Primitive.Model (see prefilterBlocks).
mkTxMeta
:: (IsOurs s Address, IsOurs s ChimericAccount, Monad m)
:: (IsOurs s Address, IsOurs s RewardAccount, Monad m)
=> TimeInterpreter m
-> BlockHeader
-> s
Expand Down Expand Up @@ -1909,7 +1908,7 @@ mkTxMeta interpretTime blockHeader wState tx cs expiry =
Just{} -> Just (fromIntegral val)
Nothing -> Nothing

ourWithdrawal :: (ChimericAccount, Coin) -> Maybe Natural
ourWithdrawal :: (RewardAccount, Coin) -> Maybe Natural
ourWithdrawal (acct, (Coin val)) =
case fst (isOurs acct wState) of
Just{} -> Just (fromIntegral val)
Expand Down Expand Up @@ -2491,7 +2490,7 @@ data ErrQuitStakePool
-- | Errors that can occur when fetching the reward balance of a wallet
data ErrFetchRewards
= ErrFetchRewardsNetworkUnreachable ErrNetworkUnavailable
| ErrFetchRewardsReadChimericAccount ErrReadChimericAccount
| ErrFetchRewardsReadRewardAccount ErrReadRewardAccount
deriving (Generic, Eq, Show)

data ErrSelectForMigration
Expand Down Expand Up @@ -2539,9 +2538,9 @@ data ErrNotASequentialWallet
= ErrNotASequentialWallet
deriving (Generic, Eq, Show)

data ErrReadChimericAccount
= ErrReadChimericAccountNotAShelleyWallet
| ErrReadChimericAccountNoSuchWallet ErrNoSuchWallet
data ErrReadRewardAccount
= ErrReadRewardAccountNotAShelleyWallet
| ErrReadRewardAccountNoSuchWallet ErrNoSuchWallet
deriving (Generic, Eq, Show)

data ErrWithdrawalNotWorth
Expand Down

0 comments on commit 9c9d134

Please sign in to comment.