Get epoch and slot lengths from HFC History Interpreter #2246

rvl · 2020-10-15T06:11:19Z

Issue Number

#2226

Overview

Remove slot length and epoch length from genesis parameters, because we will be getting them with HFC queries.
Hard-code activeSlotCoeff to 1.0 as before. There needs to be a ledger change to support this.
Add these parameters to SHELLEY_NETWORK integration test.

Anviking · 2020-10-16T08:45:02Z

👌 seems like a good idea

KtorZ · 2020-10-21T08:13:53Z

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

        , checkpointEpochStability = coerce (gp ^. #getEpochStability)
-        , checkpointActiveSlotCoeff =
-            W.unActiveSlotCoefficient (gp ^. #getActiveSlotCoefficient)
+        , checkpointActiveSlotCoeff = 0


What about renaming the fields with a Unused suffix so it is a bit more transparent / consistent with other unused fields? It's a bit sad that we can't drop columns in SQLite tables :(

KtorZ · 2020-10-21T08:25:31Z

lib/core/src/Cardano/Wallet/Network.hs

+
+    -- TODO: Query activeSlotCoeff. Where to get it from though? Need to be able
+    -- to query Globals from ledger.
+    let getActiveSlotCoeff = pure (ActiveSlotCoefficient 1.0)


We can't really hard-code this value because we use a different value for testing than what's use on the mainnet and testnet networks :/
I'd suggest to keep the active slot coefficient untouched and keep retrieving it from the Shelley genesis. The value for Byron is always known and equal to 1.0 so we could have this function return 1.0 or the shelley value depending on the era we're in?

In the meantime, let's ask if the Globals can be made available through the LSQ protocol.

In the meantime, let's ask if the Globals can be made available through the LSQ protocol.

Think Rodney already asked -> https://input-output-rnd.slack.com/archives/CFKLUH4R0/p1603176743124800, and it looked like they were going to add it.

Yes, I could add an era switch for hardcoding the mainnet activeSlotCoeff value, or we could wait for it to be implemented in shelley-ledger and cardano-node.

rvl · 2020-10-22T05:59:04Z

bors try

iohk-bors · 2020-10-22T06:50:51Z

try

Build failed:

buildkite/cardano-wallet

Is this a new one?

Failures:

  test/unit/Cardano/Wallet/Primitive/FeeSpec.hs:379:9:
  1) Cardano.Wallet.Primitive.Fee.prop_rebalanceSelection The fee balancing algorithm converges for any coin selection.
       Falsifiable (after 731 tests):
         SaveMoney
         Coin {getCoin = 183844}
         selection (before):
         inputs: - 2nd 867b7042 (~ 195781 @ 61646472...64722d30)
         withdrawal: 833505
         reclaim: -0
         outputs:
           - 76923 @ 61646472...64722d30
           - 140568 @ 61646472...64722d32
           - 2843 @ 61646472...64722d32
           - 106619 @ 61646472...64722d32
           - 64554 @ 61646472...64722d30
           - 69743 @ 61646472...64722d33
           - 192892 @ 61646472...64722d33
         change: [375144]
         deposit: -0

         selection (after):
         inputs: - 2nd 867b7042 (~ 195781 @ 61646472...64722d30)
         withdrawal: 833505
         reclaim: -0
         outputs:
           - 76923 @ 61646472...64722d30
           - 140568 @ 61646472...64722d32
           - 2843 @ 61646472...64722d32
           - 106619 @ 61646472...64722d32
           - 64554 @ 61646472...64722d30
           - 69743 @ 61646472...64722d33
           - 192892 @ 61646472...64722d33
         change: [183844]
         deposit: -0

         delta (before): 0
         delta (after):  191300
         total fee:      191300
         remaining fee:  0

  To rerun use: --match "/Cardano.Wallet.Primitive.Fee/prop_rebalanceSelection/The fee balancing algorithm converges for any coin selection./"

Randomized with seed 2023414232

Finished in 892.7483 seconds
2115 examples, 1 failure, 2 pending

KtorZ · 2020-10-22T09:24:05Z

Is this a new one?

@rvl , no, seen in the past. This property is sometimes failing on rare cases but we haven't investigated the reason yet. The result looks okay everytime, but maybe a > instead of >= in an assertion is causing this to fail.

rvl · 2020-11-04T06:34:45Z

bors r+

iohk-bors · 2020-11-04T06:34:48Z

👎 Rejected by too few approved reviews

Anviking

lgtm! Maybe a comment could be nice about #2246 (comment) if you agree.

Tried locally, on master:

$ cardano-wallet network parameters | jq .
Ok.
{
  "slot_length": {
    "quantity": 20,
    "unit": "second"
  },
  "decentralization_level": {
    "quantity": 48,
    "unit": "percent"
  },
  "epoch_stability": {
    "quantity": 2160,
    "unit": "block"
  },
  "genesis_block_hash": "5f20df933584822601f9e3f8c024eb5eb252fe8cefb24d1317dc3d432e940ebb",
  "blockchain_start_time": "2017-09-23T21:44:51Z",
  "desired_pool_number": 150,
  "epoch_length": {
    "quantity": 21600,
    "unit": "slot"
  },
  "active_slot_coefficient": {
    "quantity": 100,
    "unit": "percent"
  },
  "hardfork_at": {
    "epoch_start_time": "2020-07-29T21:44:51Z",
    "epoch_number": 208
  },
  "minimum_utxo_value": {
    "quantity": 1000000,
    "unit": "lovelace"
  }
}

and with this pr:

$ cardano-wallet network parameters | jq .
Ok.
{
  "slot_length": {
    "quantity": 1,
    "unit": "second"
  },
  "decentralization_level": {
    "quantity": 48,
    "unit": "percent"
  },
  "epoch_stability": {
    "quantity": 2160,
    "unit": "block"
  },
  "genesis_block_hash": "5f20df933584822601f9e3f8c024eb5eb252fe8cefb24d1317dc3d432e940ebb",
  "blockchain_start_time": "2017-09-23T21:44:51Z",
  "desired_pool_number": 150,
  "epoch_length": {
    "quantity": 432000,
    "unit": "slot"
  },
  "active_slot_coefficient": {
    "quantity": 100,
    "unit": "percent"
  },
  "hardfork_at": {
    "epoch_start_time": "2020-07-29T21:44:51Z",
    "epoch_number": 208
  },
  "minimum_utxo_value": {
    "quantity": 1000000,
    "unit": "lovelace"
  }
}

Anviking · 2020-11-04T09:21:46Z

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

-    let (apiNetworkParams, epochNoM) =
-            toApiNetworkParameters np { protocolParameters = pp }
+    let pastHorizon :: PastHorizonException -> IO W.SlottingParameters
+        pastHorizon _ = pure (slottingParameters np)


In the unlikely event this happens, I think we should rather do something else than using the genesis slotting parameters. Say the tip is in Shelley of the Byron;Shelley;Allegra mode, then the genesis slotting params would be misleading, no?

Seems unlikely, and maybe not the most important, but maybe add a comment about it at least?

I added a comment. It should never happen because we ask the node for slotting parameters of its current tip. That should always be within the horizon. The only time I have seen the PastHorizonException is when the node tip was at genesis.

Anviking · 2020-11-04T09:23:09Z

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

@@ -1799,8 +1799,11 @@ getNetworkParameters
    -> Handler ApiNetworkParameters
 getNetworkParameters (_block0, np, _st) nl = do


Just a thought: Maybe we should call this genesisNp or something.

Yes, good thought.

Anviking · 2020-11-04T09:26:09Z

lib/core/test/unit/Cardano/Wallet/Primitive/SyncProgressSpec.hs

            { getEpochLength = EpochLength 21600
            , getSlotLength  = SlotLength 10
-            , getGenesisBlockDate = StartTime (read "2019-11-09 16:43:02 UTC")
-            , getGenesisBlockHash = Hash ""


Divide up network parameters further. Remove slot length and epoch length from genesis parameters, because we will be getting them with HFC queries. Update all slotting parameter usages

rvl · 2020-11-06T08:35:00Z

bors r+

iohk-bors · 2020-11-06T11:49:50Z

Build succeeded:

rvl added the RESOLVING ISSUE Mark a PR as resolving issues, for auto-generated CHANGELOG label Oct 15, 2020

rvl self-assigned this Oct 15, 2020

rvl mentioned this pull request Oct 19, 2020

Add transaction TTL to payments API #2167

Merged

8 tasks

rvl force-pushed the rvl/2226/params-shelley branch 2 times, most recently from f16a092 to e5e9be9 Compare October 19, 2020 13:47

rvl marked this pull request as ready for review October 19, 2020 13:50

rvl force-pushed the rvl/2226/params-shelley branch 2 times, most recently from 47b00bc to 286c403 Compare October 20, 2020 04:31

rvl mentioned this pull request Oct 21, 2020

Incorrect mainnet network parameters returned from API #2226

Closed

KtorZ reviewed Oct 21, 2020

View reviewed changes

rvl force-pushed the rvl/2226/params-shelley branch from 286c403 to c4b172e Compare October 22, 2020 05:54

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

Try #2246:

2911792

rvl force-pushed the rvl/2226/params-shelley branch from c4b172e to cce88a5 Compare October 22, 2020 06:02

rvl force-pushed the rvl/2226/params-shelley branch from cce88a5 to ee24c89 Compare November 2, 2020 14:56

rvl requested review from Anviking and KtorZ November 4, 2020 08:37

Anviking approved these changes Nov 4, 2020

View reviewed changes

rvl added 4 commits November 6, 2020 17:54

Get epoch and slot lengths from HFC History Interpreter

a06183c

Divide up network parameters further. Remove slot length and epoch length from genesis parameters, because we will be getting them with HFC queries. Update all slotting parameter usages

Rename unused DB fields

f749645

Add epochLength and slotLength to the SHELLEY_NETWORK test

c13130a

Review rework about PastHorizonException handling

4a231af

rvl force-pushed the rvl/2226/params-shelley branch from ee24c89 to 4a231af Compare November 6, 2020 08:09

iohk-bors bot merged commit eff3c57 into master Nov 6, 2020

iohk-bors bot deleted the rvl/2226/params-shelley branch November 6, 2020 11:49

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Get epoch and slot lengths from HFC History Interpreter #2246

Get epoch and slot lengths from HFC History Interpreter #2246

rvl commented Oct 15, 2020 •

edited

Loading

Anviking commented Oct 16, 2020

KtorZ Oct 21, 2020

rvl Oct 22, 2020

KtorZ Oct 21, 2020

Anviking Oct 21, 2020

rvl Oct 22, 2020

rvl commented Oct 22, 2020

iohk-bors bot commented Oct 22, 2020 •

edited by rvl

Loading

KtorZ commented Oct 22, 2020

rvl commented Nov 4, 2020

iohk-bors bot commented Nov 4, 2020

Anviking left a comment •

edited

Loading

Anviking Nov 4, 2020

rvl Nov 6, 2020

Anviking Nov 4, 2020

rvl Nov 6, 2020

Anviking Nov 4, 2020

rvl commented Nov 6, 2020

iohk-bors bot commented Nov 6, 2020

		@@ -1799,8 +1799,11 @@ getNetworkParameters
		-> Handler ApiNetworkParameters
		getNetworkParameters (_block0, np, _st) nl = do

Get epoch and slot lengths from HFC History Interpreter #2246

Get epoch and slot lengths from HFC History Interpreter #2246

Conversation

rvl commented Oct 15, 2020 • edited Loading

Issue Number

Overview

Anviking commented Oct 16, 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

rvl commented Oct 22, 2020

iohk-bors bot commented Oct 22, 2020 • edited by rvl Loading

try

KtorZ commented Oct 22, 2020

rvl commented Nov 4, 2020

iohk-bors bot commented Nov 4, 2020

Anviking left a comment • 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

rvl commented Nov 6, 2020

iohk-bors bot commented Nov 6, 2020

rvl commented Oct 15, 2020 •

edited

Loading

iohk-bors bot commented Oct 22, 2020 •

edited by rvl

Loading

Anviking left a comment •

edited

Loading