Skip to content

Commit

Permalink
Merge #2160 #2165
Browse files Browse the repository at this point in the history
2160: Test that `removePools` does not remove pool metadata. r=KtorZ a=jonathanknowles

# Issue Number

#2154 

# Overview

This PR adds documentation and tests that `removePools` does not remove metadata.
    
Strictly speaking, when removing pools, we should also be able to remove any pool metadata that becomes unreachable as a result.

However, this is tricky to get right, since it's possible for more than one pool to share the same metadata hash.

For example, if two pools **_p_** and **_q_** issue registration certificates that share the same metadata hash **_h_**, and then we garbage collect pool **_p_**, we have to be careful to not remove the metadata corresponding to h in order to preserve the metadata for pool **_q_**.

One possible solution would be to remove metadata entries only when we can show that they are no longer reachable: when metadata entries are no longer referenced by any pool.
    
However, for the moment, we adopt the principle of never removing metadata. We can revise this decision in future if the metadata table grows too large.

2165: Bump version to v2020-09-22 r=Anviking a=Anviking

# Issue Number

Release

# Overview

- [x] Bump version using `./scripts/make_release.sh`


# Comments

<!-- Additional comments or screenshots to attach if any -->

<!-- 
Don't forget to:

 ✓ Self-review your changes to make sure nothing unexpected slipped through
 ✓ Assign yourself to the PR
 ✓ Assign one or several reviewer(s)
 ✓ Once created, link this PR to its corresponding ticket
 ✓ Assign the PR to a corresponding milestone
 ✓ Acknowledge any changes required to the Wiki
-->


Co-authored-by: Jonathan Knowles <jonathan.knowles@iohk.io>
Co-authored-by: Johannes Lund <johannes.lund@iohk.io>
Co-authored-by: Rodney Lorrimar <rodney.lorrimar@iohk.io>
  • Loading branch information
4 people committed Sep 23, 2020
3 parents ffeca1d + 0b7bd74 + 5662f5a commit fcc5afb
Show file tree
Hide file tree
Showing 19 changed files with 94 additions and 26 deletions.
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ See **Installation Instructions** for each available [release](https://github.co
> | cardano-wallet | jörmungandr (compatible versions) | cardano-node (compatible versions)
> | --- | --- | ---
> | `master` branch | [v0.9.0](https://github.com/input-output-hk/jormungandr/releases/tag/v0.9.0) | [1.20.0](https://github.com/input-output-hk/cardano-node/releases/tag/1.20.0)
> | [v2020-09-22](https://github.com/input-output-hk/cardano-wallet/releases/tag/v2020-09-22)`master` branch | [v0.9.0](https://github.com/input-output-hk/jormungandr/releases/tag/v0.9.0) | [1.19.1](https://github.com/input-output-hk/cardano-node/releases/tag/1.19.1) | [v0.9.0](https://github.com/input-output-hk/jormungandr/releases/tag/v0.9.0) | [1.20.0](https://github.com/input-output-hk/cardano-node/releases/tag/1.20.0)
> | [v2020-09-11](https://github.com/input-output-hk/cardano-wallet/releases/tag/v2020-09-11)`master` branch | [v0.9.0](https://github.com/input-output-hk/jormungandr/releases/tag/v0.9.0) | [1.19.1](https://github.com/input-output-hk/cardano-node/releases/tag/1.19.1)
> | [v2020-08-03](https://github.com/input-output-hk/cardano-wallet/releases/tag/v2020-08-03) | [v0.9.0](https://github.com/input-output-hk/jormungandr/releases/tag/v0.9.0) | [1.18.0](https://github.com/input-output-hk/cardano-node/releases/tag/1.18.0)
> | [v2020-07-28](https://github.com/input-output-hk/cardano-wallet/releases/tag/v2020-07-28) | [v0.9.0](https://github.com/input-output-hk/jormungandr/releases/tag/v0.9.0) | [1.18.0](https://github.com/input-output-hk/cardano-node/releases/tag/1.18.0)
Expand Down
2 changes: 1 addition & 1 deletion lib/cli/cardano-wallet-cli.cabal
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
name: cardano-wallet-cli
version: 2020.9.11
version: 2020.9.22
synopsis: Utilities for a building Command-Line Interfaces
homepage: https://github.com/input-output-hk/cardano-wallet
author: IOHK Engineering Team
Expand Down
2 changes: 1 addition & 1 deletion lib/core-integration/cardano-wallet-core-integration.cabal
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
name: cardano-wallet-core-integration
version: 2020.9.11
version: 2020.9.22
synopsis: Core integration test library.
description: Shared core functionality for our integration test suites.
homepage: https://github.com/input-output-hk/cardano-wallet
Expand Down
2 changes: 1 addition & 1 deletion lib/core/cardano-wallet-core.cabal
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
name: cardano-wallet-core
version: 2020.9.11
version: 2020.9.22
synopsis: The Wallet Backend for a Cardano node.
description: Please see README.md
homepage: https://github.com/input-output-hk/cardano-wallet
Expand Down
77 changes: 72 additions & 5 deletions lib/core/test/unit/Cardano/Pool/DB/Properties.hs
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@ import Cardano.Wallet.Primitive.Types
, PoolRegistrationCertificate (..)
, PoolRetirementCertificate (..)
, SlotNo (..)
, StakePoolMetadata (..)
, StakePoolTicker (..)
, getPoolCertificatePoolId
, getPoolRetirementCertificate
)
Expand Down Expand Up @@ -820,27 +822,67 @@ prop_rollbackRetirement DBLayer{..} certificates =
-- 1. We only remove data relating to the specified pools.
-- 2. We do not remove data relating to other pools.
--
-- Strictly speaking, when removing pools, we should also be able to remove any
-- pool metadata that becomes unreachable as a result.
--
-- However, this is tricky to get right, since it's possible for more than one
-- pool to share the same metadata hash.
--
-- For example, if two pools p and q issue registration certificates that share
-- the same metadata hash h, and then we garbage collect pool p, we have to be
-- careful to not remove the metadata corresponding to h in order to preserve
-- the metadata for pool q.
--
-- One possible solution would be to remove metadata entries only when we can
-- show that they are no longer reachable: when metadata entries are no longer
-- referenced by any pool.
--
-- However, for the moment, we adopt the principle of never removing metadata.
-- We can revise this decision in future if the metadata table grows too large.
--
prop_removePools
:: DBLayer IO
-> [PoolCertificate]
-> Property
prop_removePools
db@DBLayer {..} certificates =
monadicIO (setup >> prop)
DBLayer {..} certificates = checkCoverage
$ cover 8 (notNull poolsToRemove && notNull poolsToRetain)
"remove some pools and retain some pools"
$ cover 2 (null poolsToRemove)
"remove no pools"
$ cover 2 (null poolsToRetain)
"retain no pools"
$ cover 8 (notNull metadataToRetain)
"retain some metadata"
$ cover 2 (null metadataToRetain)
"retain no metadata"
$ monadicIO (setup >> prop)
where
setup = run $ atomically cleanDB

notNull = not . null

prop = do
-- Firstly, publish an arbitrary set of pool certificates:
run $ mapM_ (uncurry $ putPoolCertificate db) certificatePublications
-- Next, read the latest certificates for all pools:
run $ atomically $ forM_ certificatePublications $ \case
(publicationTime, Registration cert) -> do
-- In the case of a pool registration, we also add an
-- accompanying mock entry to the metadata table.
putPoolRegistration publicationTime cert
forM_ (view #poolMetadata cert) $ \(_, metadataHash) ->
putPoolMetadata metadataHash mockPoolMetadata
(publicationTime, Retirement cert) ->
putPoolRetirement publicationTime cert
-- Next, read the latest certificates and metadata for all pools:
poolIdsWithRegCertsAtStart <- run poolIdsWithRegCerts
poolIdsWithRetCertsAtStart <- run poolIdsWithRetCerts
poolMetadataAtStart <- Map.keysSet <$> run (atomically readPoolMetadata)
-- Next, remove a subset of the pools:
run $ atomically $ removePools $ Set.toList poolsToRemove
-- Finally, see which certificates remain:
-- Finally, see which certificates and metadata remain:
poolIdsWithRegCertsAtEnd <- run poolIdsWithRegCerts
poolIdsWithRetCertsAtEnd <- run poolIdsWithRetCerts
poolMetadataAtEnd <- Map.keysSet <$> run (atomically readPoolMetadata)
monitor $ counterexample $ T.unpack $ T.unlines
[ "All pools: "
, T.unlines (toText <$> Set.toList pools)
Expand All @@ -863,6 +905,11 @@ prop_removePools
assertWith "difference rule for retirements" $
poolIdsWithRetCertsAtStart `Set.difference` poolsToRemove
== poolIdsWithRetCertsAtEnd
-- For the moment, we never delete any metadata.
assertWith "equality rule #1 for metadata" $
poolMetadataAtEnd == poolMetadataAtStart
assertWith "equality rule #2 for metadata" $
poolMetadataAtEnd == metadataToRetain

-- The complete set of all pools.
pools = Set.fromList $ getPoolCertificatePoolId <$> certificates
Expand All @@ -873,6 +920,19 @@ prop_removePools
& L.splitAt (length pools `div` 2)
& bimap Set.fromList Set.fromList

-- For the moment, we never delete any metadata.
metadataToRetain = certificates
& mapMaybe toRegistrationCertificate
& mapMaybe (view #poolMetadata)
& fmap snd
& Set.fromList

toRegistrationCertificate
:: PoolCertificate -> Maybe PoolRegistrationCertificate
toRegistrationCertificate = \case
Registration c -> Just c
Retirement _ -> Nothing

certificatePublications
:: [(CertificatePublicationTime, PoolCertificate)]
certificatePublications =
Expand All @@ -886,6 +946,13 @@ prop_removePools
fmap (Set.fromList . fmap (view #poolId . snd) . catMaybes)
<$> atomically $ mapM readPoolRetirement $ Set.toList pools

mockPoolMetadata = StakePoolMetadata
{ ticker = StakePoolTicker "MOCK"
, name = "MOCK"
, description = Nothing
, homepage = "http://mock.pool/"
}

prop_listRegisteredPools
:: DBLayer IO
-> [PoolRegistrationCertificate]
Expand Down
2 changes: 1 addition & 1 deletion lib/jormungandr/cardano-wallet-jormungandr.cabal
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
name: cardano-wallet-jormungandr
version: 2020.9.11
version: 2020.9.22
synopsis: Wallet backend protocol-specific bits implemented using Jörmungandr
description: Please see README.md
homepage: https://github.com/input-output-hk/cardano-wallet
Expand Down
2 changes: 1 addition & 1 deletion lib/launcher/cardano-wallet-launcher.cabal
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
name: cardano-wallet-launcher
version: 2020.9.11
version: 2020.9.22
synopsis: Utilities for a building commands launcher
homepage: https://github.com/input-output-hk/cardano-wallet
author: IOHK Engineering Team
Expand Down
2 changes: 1 addition & 1 deletion lib/shelley/cardano-wallet.cabal
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
name: cardano-wallet
version: 2020.9.11
version: 2020.9.22
synopsis: Wallet backend protocol-specific bits implemented using Shelley nodes
description: Please see README.md
homepage: https://github.com/input-output-hk/cardano-wallet
Expand Down
2 changes: 1 addition & 1 deletion lib/test-utils/cardano-wallet-test-utils.cabal
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
name: cardano-wallet-test-utils
version: 2020.9.11
version: 2020.9.22
synopsis: Shared utilities for writing unit and property tests.
description: Shared utilities for writing unit and property tests.
homepage: https://github.com/input-output-hk/cardano-wallet
Expand Down
2 changes: 1 addition & 1 deletion lib/text-class/text-class.cabal
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
name: text-class
version: 2020.9.11
version: 2020.9.22
synopsis: Extra helpers to convert data-types to and from Text
homepage: https://github.com/input-output-hk/cardano-wallet
author: IOHK Engineering Team
Expand Down
2 changes: 1 addition & 1 deletion nix/.stack.nix/cardano-wallet-cli.nix

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion nix/.stack.nix/cardano-wallet-core-integration.nix

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion nix/.stack.nix/cardano-wallet-core.nix

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion nix/.stack.nix/cardano-wallet-jormungandr.nix

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion nix/.stack.nix/cardano-wallet-launcher.nix

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion nix/.stack.nix/cardano-wallet-test-utils.nix

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion nix/.stack.nix/cardano-wallet.nix

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion nix/.stack.nix/text-class.nix

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 5 additions & 5 deletions scripts/make_release.sh
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,14 @@ set -euo pipefail

################################################################################
# Release-specific parameters (Change when you bump the version)
OLD_GIT_TAG="v2020-09-09"
OLD_CABAL_VERSION="2020.9.9"
OLD_GIT_TAG="v2020-09-11"
OLD_CABAL_VERSION="2020.9.11"

GIT_TAG="v2020-09-11"
CABAL_VERSION="2020.9.11"
GIT_TAG="v2020-09-22"
CABAL_VERSION="2020.9.22"

JORM_TAG="v0.9.0"
CARDANO_NODE_TAG="1.19.1"
CARDANO_NODE_TAG="1.20.0"
################################################################################
OLD_DATE="${OLD_GIT_TAG//v}"
CHANGELOG=GENERATED_CHANGELOG.md
Expand Down

0 comments on commit fcc5afb

Please sign in to comment.