Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Account for contestation period in validators #351

Merged
merged 25 commits into from
May 19, 2022

Conversation

KtorZ
Copy link
Collaborator

@KtorZ KtorZ commented May 12, 2022

Fix #356

@KtorZ KtorZ requested review from ch1bo and abailly-iohk May 12, 2022 14:04
@KtorZ KtorZ force-pushed the ensemble/192/contestation-period branch from cc0ed45 to 3f97816 Compare May 12, 2022 14:19
@github-actions
Copy link

github-actions bot commented May 13, 2022

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-05-19 07:44:40.961358942 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 30.43 17.83
2 4936 15.61 8.55
3 5137 16.80 9.07
5 5536 27.10 14.93
10 6537 29.57 15.40
30 10539 70.39 36.10
41 12739 87.21 44.06

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 13740 23.34 12.73
2 14052 40.62 22.30
3 14543 63.11 35.03
4 14923 87.54 48.99

Cost of Close Transaction

Parties Tx size % max Mem % max CPU
1 9119 9.46 5.02
2 9445 12.25 6.51
3 9628 13.37 7.12
5 9990 15.49 8.24
10 10987 22.37 11.96
30 14917 55.99 30.04
37 16248 68.61 36.86

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 12987 8.61 4.98
2 13149 12.35 7.17
3 13183 14.94 8.63
5 13186 17.09 9.89
10 13421 25.38 14.61
50 14767 85.15 49.03
61 15068 99.28 57.13

@github-actions
Copy link

github-actions bot commented May 13, 2022

Unit Test Results

    5 files  ±0    84 suites  ±0   6m 48s ⏱️ -5s
222 tests ±0  220 ✔️ ±0  2 💤 ±0  0 ±0 

Results for commit 312ac0f. ± Comparison against base commit 9bc8a8c.

♻️ This comment has been updated with latest results.

@abailly-iohk abailly-iohk marked this pull request as ready for review May 18, 2022 10:28
@@ -546,14 +604,21 @@ finalizeTx TinyWallet{sign, getUTxO, coverFee} headState partialTx = do
Right validatedTx -> do
pure $ sign validatedTx

-- | Hardcoded grace time for close transaction to be valid.
-- TODO: replace/remove with deadline contestation
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess this TODO is moot now as we moved to a deadline already?

getContestationDeadline :: OnChainHeadState 'StClosed -> POSIXTime
getContestationDeadline
OnChainHeadState{stateMachine = Closed{closedThreadOutput = ClosedThreadOutput{closedContestationDeadline}}} =
closedContestationDeadline
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is slightly side-stepping the "non-introspectability" of the OnChainHeadState, but it's fine given the direction this is going.

, initialContestationPeriod :: OnChain.ContestationPeriod
, initialParties :: [OnChain.Party]
}
deriving (Eq, Show)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the direction in which this is going. As I said it yesterday, this is very similar to the InitObservation and I'm quite keen to unify them :)

}
utxoHash = toBuiltin $ hashTxOuts $ toList utxo

fanoutTx ::
-- | Snapshotted UTxO to fanout on layer 1
UTxO ->
-- | Everything needed to spend the Head state-machine output.
(TxIn, TxOut CtxUTxO, ScriptData) ->
UTxOWithScript ->
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could be a ClosedThreadOutput

, utxoHash
, parties
, contestationDeadline =
-- FIXME: we don't have useful functions for manipulating time on-chain seemingly
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What exactly is it to fix here? Sounds more like a complaint but slotNoToPOSIXTime works in this case?

}
, SomeMutation MutateValidityPastDeadline . ChangeValidityInterval <$> do
lb <- arbitrary
ub <- TxValidityUpperBound <$> arbitrary `suchThat` slotOverContestationDeadline
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also here.. you're on a naming roll!

@@ -35,7 +35,7 @@ import Test.QuickCheck (generate)
spec :: Spec
spec = do
it "can init and abort a head given nothing has been committed" $
failAfter 10 $
failAfter 20 $
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did this become slower?

Comment on lines +303 to +310
let expectedOutputDatum =
Closed
{ parties
, snapshotNumber = 0
, utxoHash = initialUtxoHash
, contestationDeadline = makeContestationDeadline cperiod ctx
}
in checkHeadOutputDatum ctx expectedOutputDatum
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Introducing a variable for this feels a bit too verbose?

Suggested change
let expectedOutputDatum =
Closed
{ parties
, snapshotNumber = 0
, utxoHash = initialUtxoHash
, contestationDeadline = makeContestationDeadline cperiod ctx
}
in checkHeadOutputDatum ctx expectedOutputDatum
checkHeadOutputDatum ctx $
Closed
{ parties
, snapshotNumber = 0
, utxoHash = initialUtxoHash
, contestationDeadline = makeContestationDeadline cperiod ctx
}

| otherwise = traceError "negative snapshot number"
{-# INLINEABLE checkClose #-}

-- | Checks that the datum
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that the datum.. ?

millisToPico = (* millisInPico)

millisInPico :: Integer
millisInPico = 10 ^ (9 :: Integer)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Name wrong way around? isn't it 10^9 pico seconds in a millisecond?

abailly-iohk and others added 23 commits May 19, 2022 09:28
Also enhance cardano-api with pinned era types for lower and upper bound
Currently set to arbitrary value as it's not really tested
We should probably drop this test anyway
One mutation for close is still failing, on purpose
@ch1bo ch1bo force-pushed the ensemble/192/contestation-period branch from d447fc2 to 312ac0f Compare May 19, 2022 07:28
@ch1bo ch1bo merged commit b73e2cb into master May 19, 2022
@ch1bo ch1bo deleted the ensemble/192/contestation-period branch May 19, 2022 09:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Close / contest validators enforcing contestation period
3 participants