Hardfork countdown support #1932

paweljakubas · 2020-07-20T12:36:09Z

Issue Number

#1917

Overview

I have extended ApiNetworkParameters
I have updated swagger accordingly
I have updated core TypeSpec
I have extended NetworkParameters
I have extended DB schema and addressed DB migrations
I have used QueryAnytimeShelley EraStart from CardanoQuery to get value
I have tested the above

Comments

To move forward I need #1909 to be merged.
As,

QueryAnyTimeShelley EraStart :: Query CardanoBlock

Anviking · 2020-07-20T17:54:25Z

lib/core/src/Cardano/Wallet/Api/Types.hs

+        (ApiEraTransitionInfo $ ApiT info)
+        (Just $ ApiEpochInfo
+            (ApiT epochN)
+            (Slotting.epochStartTime (Slotting.slotParams gp) epochN)


Be aware of this: #1913 (comment)

paweljakubas · 2020-07-22T13:23:34Z

bors try

iohk-bors · 2020-07-22T14:34:14Z

try

Build failed

buildkite/cardano-wallet

paweljakubas · 2020-07-26T21:03:33Z

bors try

iohk-bors · 2020-07-26T22:03:25Z

try

Build failed

buildkite/cardano-wallet

KtorZ · 2020-07-25T12:47:50Z

lib/core/src/Cardano/Wallet/Api/Types.hs

+    } deriving (Eq, Generic, Show)
+
+data ApiEraTransition = ApiEraTransition
+    { transition :: !ApiEraTransitionInfo


Why not simply "ApiT EraTransitionInfo" here? What does the indirection gives us?

this was removed on saturday, I do not know how you saw it as I squashed all commits before.

KtorZ · 2020-07-26T22:40:54Z

lib/core/src/Cardano/Wallet/Api/Types.hs

+    toApiEpochNo epochN =
+        ApiEpochInfo
+        (ApiT epochN)
+        (Slotting.epochStartTime (Slotting.slotParams gp) epochN)


This needs to use the time interpreter.

done in 35f6385

KtorZ · 2020-07-26T22:42:34Z

lib/core/src/Cardano/Wallet/DB/Sqlite.hs

+        addColumn conn (DBField ProtocolParametersHardforkEpoch) value
+      where
+        value = case defaultHardforkEpoch defaultFieldValues of
+            Nothing -> "NULL"


Does this work fine? Have you run the automated migration tests?

so I run :

nix-build nix/migration-tests.nix -o migration-tests ./migration-tests/runall.sh

and I see some failures:

[cardano-wallet.pools-db:Error:13] [2020-07-27 14:01:13.46 UTC] Failed to migrate the database: PersistError "\n\nDatabase migration: manual intervention required.\nThe unsafe actions are prefixed by '***' below:\n\n CREATE TEMP TABLE \"pool_owner_backup\"(\"pool_id\" VARCHAR NOT NULL,\"slot\" INTEGER NOT NULL,\"slot_internal_index\" INTEGER NOT NULL,\"pool_owner\" VARCHAR NOT NULL,\"pool_owner_index\" INTEGER NOT NULL, PRIMARY KEY (\"pool_id\",\"slot\",\"slot_internal_index\",\"pool_owner\",\"pool_owner_index\"));\n INSERT INTO \"pool_owner_backup\"(\"pool_id\",\"pool_owner\",\"pool_owner_index\") SELECT \"pool_id\",\"pool_owner\",\"pool_owner_index\" FROM \"pool_owner\";\n DROP TABLE \"pool_owner\";\n CREATE TABLE \"pool_owner\"(\"pool_id\" VARCHAR NOT NULL,\"slot\" INTEGER NOT NULL,\"slot_internal_index\" INTEGER NOT NULL,\"pool_owner\" VARCHAR NOT NULL,\"pool_owner_index\" INTEGER NOT NULL, PRIMARY KEY (\"pool_id\",\"slot\",\"slot_internal_index\",\"pool_owner\",\"pool_owner_index\"), CONSTRAINT \"pool_ownerfk_registration_pool_id\" FOREIGN KEY(\"pool_id\",\"slot\",\"slot_internal_index\") REFERENCES \"pool_registration\"(\"pool_id\",\"slot\",\"slot_internal_index\") ON DELETE CASCADE);\n INSERT INTO \"pool_owner\" SELECT \"pool_id\",\"slot\",\"slot_internal_index\",\"pool_owner\",\"pool_owner_index\" FROM \"pool_owner_backup\";\n DROP TABLE \"pool_owner_backup\";\n CREATE TEMP TABLE \"pool_registration_backup\"(\"pool_id\" VARCHAR NOT NULL,\"slot\" INTEGER NOT NULL,\"slot_internal_index\" INTEGER NOT NULL,\"margin_numerator\" INTEGER NOT NULL,\"margin_denominator\" INTEGER NOT NULL,\"cost\" INTEGER NOT NULL,\"pledge\" INTEGER NOT NULL,\"metadata_url\" VARCHAR NULL,\"metadata_hash\" VARCHAR NULL, PRIMARY KEY (\"pool_id\",\"slot\",\"slot_internal_index\"));\n INSERT INTO \"pool_registration_backup\"(\"pool_id\",\"slot\",\"cost\") SELECT \"pool_id\",\"slot\",\"cost\" FROM \"pool_registration\";\n*** DROP TABLE \"pool_registration\";\n CREATE TABLE \"pool_registration\"(\"pool_id\" VARCHAR NOT NULL,\"slot\" INTEGER NOT NULL,\"slot_internal_index\" INTEGER NOT NULL,\"margin_numerator\" INTEGER NOT NULL,\"margin_denominator\" INTEGER NOT NULL,\"cost\" INTEGER NOT NULL,\"pledge\" INTEGER NOT NULL,\"metadata_url\" VARCHAR NULL,\"metadata_hash\" VARCHAR NULL, PRIMARY KEY (\"pool_id\",\"slot\",\"slot_internal_index\"));\n INSERT INTO \"pool_registration\" SELECT \"pool_id\",\"slot\",\"slot_internal_index\",\"margin_numerator\",\"margin_denominator\",\"cost\",\"pledge\",\"metadata_url\",\"metadata_hash\" FROM \"pool_registration_backup\";\n DROP TABLE \"pool_registration_backup\";\n CREATE TABLE \"pool_retirement\"(\"pool_id\" VARCHAR NOT NULL,\"slot\" INTEGER NOT NULL,\"slot_internal_index\" INTEGER NOT NULL,\"epoch\" INTEGER NOT NULL, PRIMARY KEY (\"pool_id\",\"slot\",\"slot_internal_index\"));\n CREATE TABLE \"pool_metadata\"(\"metadata_hash\" VARCHAR NOT NULL,\"name\" VARCHAR NOT NULL,\"ticker\" VARCHAR NOT NULL,\"description\" VARCHAR NULL,\"homepage\" VARCHAR NOT NULL, PRIMARY KEY (\"metadata_hash\"));\n CREATE TABLE \"pool_metadata_fetch_attempts\"(\"metadata_hash\" VARCHAR NOT NULL,\"metadata_url\" VARCHAR NOT NULL,\"retry_after\" TIMESTAMP NOT NULL,\"retry_count\" INTEGER NOT NULL, PRIMARY KEY (\"metadata_hash\",\"metadata_url\"));\n"

also

[cardano-wallet.pools-db:Error:13] [2020-07-27 14:00:50.77 UTC] Failed to migrate the database: : NOT NULL constraint failed: pool_owner_backup.slot

BUT do not see problems with hardfork_epoch . We will deal with this in separate PR to my best knowledge

KtorZ · 2020-07-26T22:42:58Z

lib/core/src/Cardano/Wallet/DB/Sqlite/Types.hs

+    fromPersistValue = fromPersistValue >=> mkEpochNo
+
+instance PersistFieldSql EpochNo where
+    sqlType _ = sqlType (Proxy @Text)


🤔 Why is the epoch number stored as a Text ? Wouldn't something like a Word32 or Word64 be more suitable (especially for things like filtering and where clauses)?

frankly speaking I was here mimicking how we handled this for SlotNo

We do store SlotNo as Word64 though 😅

deriving via Word64 instance (PersistFieldSql SlotNo)

yes, very true -> done in a3a5f8f

KtorZ · 2020-07-26T23:16:18Z

lib/shelley/src/Cardano/Wallet/Shelley/Network.hs

+                        Right boundM ->
+                            onPParamsUpdate' $ convert boundM ls
+                        Left (e2 :: AcquireFailure) ->
+                            traceWith tr $ MsgLocalStateQueryError TipSyncClient $ show e2


Hmmm. The logic feels sort of weird / intertwined here. Query data is mixed with handling results which makes the whole a bit hard to follow (+ causes some logic duplication in the error handling).

What about:

run the GetCurrentPParams & GetEraStart in "parallel"

combine them via an applicative together in handleParamsUpdate

So something along the lines of:

queryLocalState :: Point (CardanoBlock sc) -> m () queryLocalState pt = do mb <- localStateQueryQ `send` CmdQueryLocalState pt (QueryAnytimeShelley GetEraStart) pp <- localStateQueryQ `send` CmdQueryLocalState pt (QueryIfCurrentShelley Shelley.GetCurrentPParams) sequence (handleParamsUpdate fromShelleyPParams <$> mb <*> pp) >>= handleAcquireFailure st <- localStateQueryQ `send` CmdQueryLocalState pt (QueryIfCurrentByron Byron.GetUpdateInterfaceState) sequence (handleParamsUpdate protocolParametersFromUpdateState <$> mb <*> st) >>= handleAcquireFailure handleAcquireFailure :: Either AcquireFailure () -> m () handleAcquireFailure = \case Right () -> pure () Left e -> traceWith tr $ MsgLocalStateQueryError TipSyncClient $ show e handleParamsUpdate :: (Maybe Bound -> p -> W.ProtocolParameters) -> (Maybe Bound) -> (Either (MismatchEraInfo (CardanoEras sc)) p) -> m () handleParamsUpdate convert boundM = \case Right ls -> do onPParamsUpdate' $ convert boundM ls Left mismatch -> do traceWith tr $ MsgLocalStateQueryEraMismatch mismatch

yes, it is much better. I was planning to do it after proof of concept solution

done in 611ac05

paweljakubas · 2020-07-27T15:34:39Z

I will need to wait for mainnet test when #1943 is merged - to my best knowledge

KtorZ · 2020-07-27T15:35:29Z

lib/core-integration/src/Test/Integration/Scenario/API/Shelley/Network.hs

@@ -46,4 +48,5 @@ spec = do
            [ expectField (#decentralizationLevel) (`shouldBe` d)
            , expectField (#desiredPoolNumber) (`shouldBe` nOpt)
            , expectField (#minimumUtxoValue) (`shouldBe` minUtxoValue)
+            , expectField (#hardforkEpochInfo) (`shouldNotBe` Nothing)


Will this one be ever true? Since we only run the test scenarios AFTER the hard-fork has happened, I'd assume that Nothing will be returned forever. No ?

it was Nothing...if you run this test once! ie., one has to wait for tip client waiting to make local query

stack test --ta '-m "NETWORK_PARAMS - Able to fetch network parameters"' cardano-wallet-shelley:integration

But when running several tests then I have Just ... so I used eventually

KtorZ · 2020-07-27T15:38:49Z

lib/core/src/Cardano/Wallet/DB/Sqlite/Types.hs

+    where c = unsafeEpochNo n
+
+persistEpochNo :: EpochNo -> PersistValue
+persistEpochNo = toPersistValue . fromIntegral @Word31 @Word32 . unEpochNo


Looks better 😄

KtorZ · 2020-07-27T15:42:19Z

specifications/api/swagger.yaml

@@ -848,6 +848,7 @@ components:
        decentralization_level: *percentage
        desired_pool_number: *stakePoolsNumber
        minimum_utxo_value: *amount
+        hardfork_epoch_info: *epochInfo


What about just hardfork ? or hardfork_at for consistency across the API namings?

ok, will change very soon

done in ed5a885

paweljakubas · 2020-07-27T18:37:31Z

setup: cardano-wallet-shelley produced with this branch and connected to the mainnet
before 18:30 UTC :

cardano-wallet-shelley network parameters
no  "hardfork_at" in response

after 18:30 UTC:

     "hardfork_at": {
        "epoch_start_time": "2020-07-29T21:44:51Z",
        "epoch_number": 208
    },

Thanks @Anviking and @piotr-iohk with helping in verifying this!

extend swagger add testing and correct swagger introduce EraTransitionInfo in Primitive.Types extend NetworkParameters change ApiNetworkParameter to be not list but potential value extend db schema and migration plus simplify usage of EraTransition simplify the stuff

KtorZ · 2020-07-27T19:21:19Z

bors r+

1932: Hardfork countdown support r=KtorZ a=paweljakubas # Issue Number  #1917 # Overview  - [x] I have extended `ApiNetworkParameters` - [x] I have updated swagger accordingly - [x] I have updated core `TypeSpec` - [x] I have extended `NetworkParameters` - [x] I have extended DB schema and addressed DB migrations - [x] I have used `QueryAnytimeShelley EraStart` from `CardanoQuery` to get value - [x] I have tested the above # Comments  To move forward I need #1909 to be merged. As, ``` QueryAnyTimeShelley EraStart :: Query CardanoBlock ```  Co-authored-by: Pawel Jakubas <[email protected]>

iohk-bors · 2020-07-27T20:56:27Z

Build failed

buildkite/cardano-wallet

KtorZ · 2020-07-27T21:05:11Z

Unit tests running OOM. Reported in the spreadsheet, hopefully fixed by #1954.

paweljakubas added the ADDING FEATURE Mark a PR as adding a new feature, for auto-generated CHANGELOG label Jul 20, 2020

paweljakubas added this to the (ADP-245) Shelley activation countdown milestone Jul 20, 2020

paweljakubas self-assigned this Jul 20, 2020

Anviking reviewed Jul 20, 2020

View reviewed changes

paweljakubas force-pushed the paweljakubas/1917/hardfork-countdown branch from 9fcc194 to bb4a45e Compare July 22, 2020 11:14

iohk-bors bot added a commit that referenced this pull request Jul 22, 2020

Try #1932:

833fab7

paweljakubas force-pushed the paweljakubas/1917/hardfork-countdown branch 5 times, most recently from c4f1e12 to ffe5e25 Compare July 26, 2020 21:03

iohk-bors bot added a commit that referenced this pull request Jul 26, 2020

Try #1932:

7815cfa

paweljakubas marked this pull request as ready for review July 26, 2020 21:03

KtorZ reviewed Jul 26, 2020

View reviewed changes

paweljakubas force-pushed the paweljakubas/1917/hardfork-countdown branch from ffe5e25 to 611ac05 Compare July 27, 2020 09:01

KtorZ reviewed Jul 27, 2020

View reviewed changes

paweljakubas force-pushed the paweljakubas/1917/hardfork-countdown branch from a3a5f8f to a1b4f57 Compare July 27, 2020 15:35

KtorZ reviewed Jul 27, 2020

View reviewed changes

paweljakubas requested review from KtorZ and Anviking July 27, 2020 18:42

paweljakubas added 3 commits July 27, 2020 21:11

small fixes after rebase

ac4a9d0

use QueryAnytimeShelley GetEraStart

d45b609

paweljakubas added 7 commits July 27, 2020 21:11

use information from localqueryprotocol in protocol updating

82e8b58

add test in network scenario of shelley

1ac1191

adopt sequence mode when getting era start with network parameters

374695d

use timeInterpreter rather than pure functions from Slotting

f05914e

change test to detect hardfork in network parameters

0199e11

change to PersistFieldSql of epochNo

a99844a

change name of ApiNetworkParameters field

1dc53db

paweljakubas force-pushed the paweljakubas/1917/hardfork-countdown branch from ed5a885 to 1dc53db Compare July 27, 2020 19:12

KtorZ approved these changes Jul 27, 2020

View reviewed changes

KtorZ merged commit 7d07428 into master Jul 27, 2020

KtorZ deleted the paweljakubas/1917/hardfork-countdown branch July 27, 2020 21:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Hardfork countdown support #1932

Hardfork countdown support #1932

paweljakubas commented Jul 20, 2020 •

edited

Loading

Anviking Jul 20, 2020

paweljakubas commented Jul 22, 2020

iohk-bors bot commented Jul 22, 2020

paweljakubas commented Jul 26, 2020

iohk-bors bot commented Jul 26, 2020

KtorZ Jul 25, 2020

paweljakubas Jul 27, 2020

KtorZ Jul 26, 2020

paweljakubas Jul 27, 2020

KtorZ Jul 26, 2020

paweljakubas Jul 27, 2020 •

edited

Loading

KtorZ Jul 26, 2020

paweljakubas Jul 27, 2020

KtorZ Jul 27, 2020

paweljakubas Jul 27, 2020

KtorZ Jul 26, 2020

paweljakubas Jul 27, 2020

paweljakubas Jul 27, 2020

paweljakubas commented Jul 27, 2020

KtorZ Jul 27, 2020

paweljakubas Jul 27, 2020

KtorZ Jul 27, 2020

KtorZ Jul 27, 2020

paweljakubas Jul 27, 2020

paweljakubas Jul 27, 2020

paweljakubas commented Jul 27, 2020 •

edited

Loading

KtorZ commented Jul 27, 2020

iohk-bors bot commented Jul 27, 2020

KtorZ commented Jul 27, 2020

Hardfork countdown support #1932

Hardfork countdown support #1932

Conversation

paweljakubas commented Jul 20, 2020 • edited Loading

Issue Number

Overview

Comments

Choose a reason for hiding this comment

paweljakubas commented Jul 22, 2020

iohk-bors bot commented Jul 22, 2020

try

Build failed

paweljakubas commented Jul 26, 2020

iohk-bors bot commented Jul 26, 2020

try

Build failed

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

paweljakubas Jul 27, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

paweljakubas commented Jul 27, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

paweljakubas commented Jul 27, 2020 • edited Loading

KtorZ commented Jul 27, 2020

iohk-bors bot commented Jul 27, 2020

Build failed

KtorZ commented Jul 27, 2020

paweljakubas commented Jul 20, 2020 •

edited

Loading

paweljakubas Jul 27, 2020 •

edited

Loading

paweljakubas commented Jul 27, 2020 •

edited

Loading