Extract communication independent parts from Direct chain

[ghost]

This is a minor PR that splits the large H.C.Direct module in two: One part dealing with Ouroboros and node interaction through mini-protocols, another part which only takes care of transactions transformation from/to chain. This need arose while working on #375 in order to be able to implement a chain component that could emulate the actual behaviour of a chain without having to implement the protocol logic, so that it can be run within IOSim monad.

[github-actions]

Unit Test Results

    5 files  ±0    83 suites  ±0   7m 53s ⏱️ +28s
224 tests ±0  222 ✔️ ±0  2 💤 ±0  0 ❌ ±0 

Results for commit c634b93. ± Comparison against base commit f54c069.

♻️ This comment has been updated with latest results.

[github-actions]

Transactions Costs

Sizes and execution budgets for Hydra protocol transactions. Note that unlisted parameters are currently using arbitrary values and results are not fully deterministic and comparable to previous runs.

Metadata
Generated at	2022-06-01 16:22:10.068922325 UTC
Max. memory units	14000000.00
Max. CPU units	10000000000.00
Max. tx size (kB)	16384

Cost of Init Transaction

Parties	Tx size	% max Mem	% max CPU
1	4736	16.49	9.30
2	4936	24.34	13.88
3	5138	16.05	8.62
5	5538	35.40	20.03
10	6538	51.31	28.69
30	10540	71.21	36.60
42	12939	88.53	44.66

Cost of Commit Transaction

Currently only one UTxO per commit allowed (this is about to change soon)

UTxO	Tx size	% max Mem	% max CPU
1	5862	17.98	8.81

Cost of CollectCom Transaction

Parties	Tx size	% max Mem	% max CPU
1	13442	19.08	10.22
2	14360	41.56	22.84
3	14422	57.11	31.46
4	14954	83.13	46.15

Cost of Close Transaction

Parties	Tx size	% max Mem	% max CPU
1	9469	10.53	5.61
2	9391	8.63	4.58
3	9767	11.89	6.33
5	10274	16.27	8.70
10	11566	28.91	15.47
30	15293	58.82	31.58
55	16209	69.50	36.59

Cost of Contest Transaction

Parties	Tx size	% max Mem	% max CPU
1	9420	10.08	5.37
2	9468	9.80	5.21
3	9788	12.24	6.51
5	10395	18.40	9.83
10	11409	25.68	13.73
30	15028	52.16	27.98
36	16176	61.43	32.98

Cost of Abort Transaction

Parties	Tx size	% max Mem	% max CPU

Cost of FanOut Transaction

Involves spending head output and burning head tokens. Uses ada-only UTxO for better comparability.

UTxO	Tx size	% max Mem	% max CPU
1	13366	11.47	6.65
3	13370	13.65	7.90
5	13438	15.66	9.02
10	13542	23.31	13.42
59	15187	96.45	55.51

[ch1bo]

Not fully convinced on this "separation of concerns". Although you already gave a bit of context that you see it work out better in some testing scenario, can you point me to something enlightening having the cut done like this is particularly useful?

Maybe it's just the inner postTx bits (fromPostChainTx + finalize) what you are seeking for?

I guess my hesitation is that the Handlers' implementation of the Chain handle does not actually "post" a tx?

[ch1bo]

What a weird interface we have there with the queue. I see it's unchanged in your refactor and it seems to be enqueuing the tx for submission and blocking on a response by the node. Hence, it should be the responsibility of this module.. not the Handlers one?

I would propose to not use the Chain interface in this Handlers module, but have a function which returns the Tx instead of the current Chain.postTx interface and do the enqueuing / waiting for errors in this module up here. i.e. something like

Suggested change

	action $ mkChain tracer (queryTimeHandle networkId socketPath) cardanoKeys wallet headState queue
	action $
	Chain
	{ postTx = \tx -> do
	vtx <- Handlers.createTx tracer (queryTimeHandle networkId socketPath) cardanoKeys wallet headState tx
	response <- newEmptyTMVar
	writeTQueue queue (vtx, response)
	atomically (takeTMVar response) >>= \case
	Nothing -> pure ()
	Just err -> throwIO err
	}

[ghost]

I would kind of agree but for the fact I really wanted to do this refactoring to be able to test this very function in an isolated way, or rather independently from the underlying network machinery.

[ch1bo]

Ah so you mean it's important to you that a Chain interface is exposed by the Handlers module? I can live with this change overall, but want to state that while it moves things a bit out of the way, it certainly is not easier to understand with the TMVar in the TQueue.

[ghost]

I guess this would make more sense if I had implemented the mock chain I am envisioning that would use this module, but this would take more time and this change is self-contained and in my opinion improves on the current situation: At least, the "handlers" are now testable without requiring a full node. Perhaps expressing this kind of tests would help motivating it even more?

[ch1bo]

I can live with this change overall, but want to state that while it moves things a bit out of the way, it certainly is not easier to understand with the TMVar in the TQueue.

[ghost]

I agree @ch1bo , I am thinking of enhancing the PR with a cleaner interface. Give me a couple hours

[ch1bo]

Already better, although I think it could still be made simpler. Still approved 🙂

[ch1bo]

I think it could be made even simpler by just providing the ValidatedTx? i.e. treat this thing as a callback: type TxPoster = ValidatedTx Era -> m ()?

[ghost]

I tried but this would be a larger change: The STM action is wrapped inside another STM action in the called in order to provide a synchronous interface, by creating and passing the TMVar to hold the result. I tried to change this logic to something simpler but quite a few tests fail and did not want to spend the time to investigate.