LocalStateQuery: Use params from chain when building txs

[rvl]

Context

Part of ADP-83.

The wallet server must keep track of certain protocol parameters that may change through soft protocol updates. The current parameters of interest both relate to transaction construction:

• max tx size
• fee policy

These two parameters are needed when building a transaction before running the coin selection.

Relevant parameter functions are:

byronGenesisConfig
  :: Ouroboros.Consensus.Byron.Ledger.Config.BlockConfig
  -> Cardano.Chain.Genesis.Config

configGenesisData :: Cardano.Chain.Genesis.Config -> GenesisData

gdProtocolParameters
  :: Cardano.Chain.Genesis.GenesisData
  -> Cardano.Chain.Update.ProtocolParameters

ppMaxTxSize :: ProtocolParameters -> Natural

ppTxFeePolicy :: ProtocolParameters -> TxFeePolicy

The state machine types are in Ouroboros.Network.Protocol.LocalStateQuery.Client.
With cardano-node since revision 0172a96e16f46f1688da6dd8294481bb995fd72d, a new entry is added to NodeToClientProtocols that we can use. In our network layer Cardano.Wallet.Byron.Network we need to make room for this protocol.

Example code exists in Ouroboros.Network.Protocol.LocalStateQuery.Example:

localStateQueryClient
  :: forall block query result m. Applicative m
  => [(Point block, query result)]
  -> LocalStateQueryClient block query m
                           [(Point block, Either AcquireFailure result)]

In the wallet engine are already blockchain parameters stored in the database with every wallet checkpoint.

Note that for transaction construction, only the parameters for the network tip are relevant. A transaction obviously cannot be inserted into the chain under the network tip.

The node only maintains local state for the last k blocks from its tip.

The type Cardano.Chain.Update.ProtocolParameters is the values that the node supports updates for. The intersection of this type and Cardano.Wallet.Primitive.Types.BlockchainParameters is the following three fields:
- ppSlotDuration (getSlotLength)
- ppMaxTxSize
- ppTxFeePolicy

Decision

Option A - parameters from checkpoint

The present code structure has the full bundle of parameters stored in each wallet checkpoint. So it's convenient to use the parameters from the wallet tip (not network tip) when constructing a transaction.

So, for each wallet, after fetching the latest blocks, use LocalStateQuery to get the blockchain parameters for the wallet tip.

Save these dynamic blockchain parameters into the wallet checkpoint after applying the latest blocks.

Option B - request latest parameters when constructing tx

Add a method to NetworkLayer something like this

tipBlockchainParameters
  :: NetworkLayer
  -> ExceptT ErrTipBlockchainParameters m BlockchainParameters

Use this function before running coin selection to get the tx max size and fee policy.

Possible problem

The LocalStateQuery client is not synchronised with the ChainSync client. So there is a race condition between getting the tip point and querying the parameters for this point.

Acceptance Criteria

1. Transactions are created using parameters dynamically updated from the chain.

---

Development

1. Go with something like option B.
2. Make a specialised chain sync + local state query node client that just keeps the tip point and latest parameters in a mutable variable. This state is contained inside the network layer.
3. Split BlockchainParameters into the parameters that always stay the same and the parameters that can be updated. The update parameters are not associated with any particular checkpoint, just the latest node tip.
4. Modify the database so that the split BlockchainParameters can be saved and loaded.
5. When creating transactions, use the update parameters loaded from the database.

QA

1. There is a new unit test Cardano.Wallet.Byron.NetworkSpec.spec which tests querying protocol parameters from the node.
2. Existing integration tests can check that TxParameters from node are being used in transactions -- because initial TxParameters are forced to zero in lib/byron/test/integration/Main.hs:182.
3. I was thinking about using cardano-cli to submit an update proposal to test TxParameter changes. But this would probably result in a slow flaky test.

DB Migrations

There will be a new tx_parameters table added, but no other fields are modified. So automatic migrations will be ok.

[piotr-iohk]

@rvl looks good, I just have few questions:

There will be a new tx_parameters table added, but no other fields are modified. So automatic migrations will be ok.

Not sure if it is possibly related but I raised DB migration issue -> #1649

Existing integration tests can check that TxParameters from node are being used in transactions -- because initial TxParameters are forced to zero in lib/byron/test/integration/Main.hs:182

Does that mean we could/should update existing integration tests for transactions to verify this (or add some new integration test checking this)

3. Should it affect maybe GET /network/parameters (in terms of returned data perhaps?)

[rvl]

Thanks @piotr-iohk

1. Not sure if it is possibly related but I raised DB migration issue -> DB migration tests failing intermittently #1649

I do not think it's related. The two causes are port conflicts and jormungandr changes. There was one migration test failure because I removed some table columns, but they started passing again once I put the columns back.

2. Does that mean we could/should update existing integration tests for transactions to verify this (or add some new integration test checking this)

I decided that a parameter change integration test will be slow and flaky.

Something I could possibly do for the integration tests is set the initial TxParameters to dummy values. Then the transaction tests will show that the parameters did successfully update once. But I did not do that because it adds a bit of "wtf?" code to test/integration/Main.hs.

3. Should it affect maybe GET /network/parameters (in terms of returned data perhaps?)

The two tx parameters were never included in this endpoint, so it stays exactly the same.

[piotr-iohk]

ok, closing. 👍