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

HF integration #1112

Merged
merged 19 commits into from
Jun 6, 2022
Merged

HF integration #1112

merged 19 commits into from
Jun 6, 2022

Conversation

kderme
Copy link
Contributor

@kderme kderme commented May 5, 2022

#1081 and #1120

@kderme kderme requested a review from erikd as a code owner May 5, 2022 22:12
@kderme kderme force-pushed the kderme/hf-integration branch 4 times, most recently from dc33deb to 9c5c1ad Compare May 13, 2022 12:28
@kderme kderme force-pushed the kderme/hf-integration branch 2 times, most recently from e8077b7 to 77d5626 Compare May 24, 2022 04:25
@kderme kderme force-pushed the kderme/hf-integration branch 2 times, most recently from 1f0a861 to 05088fc Compare May 24, 2022 08:44
@kderme kderme force-pushed the kderme/hf-integration branch 2 times, most recently from bd15a2d to ce97d18 Compare May 26, 2022 13:38
@kderme kderme mentioned this pull request May 26, 2022
import Cardano.Mock.Forging.Tx.Alonzo.ScriptsExamples
import Cardano.Mock.Forging.Tx.Generic
import Cardano.Mock.Forging.Types
-- import qualified Data.Maybe as Strict
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
-- import qualified Data.Maybe as Strict

{-# LANGUAGE TupleSections #-}
{-# LANGUAGE TypeApplications #-}

{-# OPTIONS_GHC -Wno-orphans #-}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
{-# OPTIONS_GHC -Wno-orphans #-}

Seems to compile without this option.

, _poolRAcnt = RewardAcnt Testnet rwCred
, _poolOwners = Set.fromList owners
, _poolRelays = StrictSeq.singleton $ SingleHostAddr Strict.SNothing Strict.SNothing Strict.SNothing
, _poolMD = Strict.SJust $ PoolMetadata (fromJust $ textToUrl "best.pool") "89237365492387654983275634298756"
Copy link
Contributor

Choose a reason for hiding this comment

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

A comment saying where these values come from and whether they are special in some way, would be helpful.

import Cardano.Mock.Forging.Tx.Generic
import Cardano.Mock.Forging.Types

delegateAndSendBlocks :: Int -> Interpreter -> IO [CardanoBlock]
Copy link
Contributor

Choose a reason for hiding this comment

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

A comment describing what this is doing and why would be useful.


{-# OPTIONS_GHC -Wno-orphans #-}

module Cardano.Mock.Forging.Tx.Babbage where
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd prefer a nice export list here with some comments but I see that isn't the case for other Eras.

stats <- liftIO $ textShowStats cache
liftIO . logInfo tracer $ stats
liftIO . logInfo tracer $ "Starting epoch " <> textShow (unEpochNo en)
LedgerStartAtEpoch en ->
-- This is different from the previous case in that the db-sync started
-- in this epoch, for example after a restart, instead of after an epoch boundary.
liftIO . logInfo tracer $ "Starting at epoch " <> textShow (unEpochNo en)
LedgerDeltaRewards rwd -> do
LedgerDeltaRewards _e rwd -> do
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
LedgerDeltaRewards _e rwd -> do
LedgerDeltaRewards _ rwd -> do

blockHash =
Crypto.hashToBytes . Protocol.unHashHeader
Crypto.hashToBytes -- . Protocol.unHashHeader
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Crypto.hashToBytes -- . Protocol.unHashHeader
Crypto.hashToBytes

@@ -49,8 +49,9 @@ ledgerAddrBalance addr lsc =
LedgerStateAllegra st -> getShelleyBalance addr $ getUTxO st
LedgerStateMary st -> getShelleyBalance addr $ getUTxO st
LedgerStateAlonzo st -> getAlonzoBalance addr $ getUTxO st
LedgerStateBabbage _st -> panic "undefined Babbage ledgerAddrBalance"
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be using the existing Either Text Word64 for signalling an un-expected return.
The calling code in Cardano.DbTool.Validate.Ledger logs the result on Left.

Suggested change
LedgerStateBabbage _st -> panic "undefined Babbage ledgerAddrBalance"
LedgerStateBabbage _st -> Left "undefined Babbage ledgerAddrBalance"

pure (eutxo, maTxOuts)
where
hasScript :: Bool
hasScript = maybe False Generic.hasCredScript (Generic.getPaymentCred addr)

inertCollateralTxOut
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
inertCollateralTxOut
insertCollateralTxOut

Left (_err, bs) -> insertStakeAddress txId rewardAddr (Just bs)
Right addrId -> pure addrId

-- If the address already esists in the table, it will not be inserted again (due to
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
-- If the address already esists in the table, it will not be inserted again (due to
-- If the address already exists in the table, it will not be inserted again (due to

@kderme kderme force-pushed the kderme/hf-integration branch 2 times, most recently from 9e20208 to 5613022 Compare June 2, 2022 17:48
kderme added 6 commits June 3, 2022 15:23
This avoids many transformations from ledger types, but more
importantly ledger types are backed by ShortByteString which
is not pinned memory and doesn't cause heap fragmentation.
Using the whole CostModel caused errors because the unique
key can't be that big.
Rollbacks are extremely slow without them
@erikd erikd force-pushed the kderme/hf-integration branch from 5613022 to 9f24209 Compare June 6, 2022 05:34
Mainly:
* Use explicit export lists where possible.
* Use explicit or qualified imports for all imports not in this repo.
@erikd erikd force-pushed the kderme/hf-integration branch from 9f24209 to 10868e0 Compare June 6, 2022 05:35
@erikd erikd merged commit bc6ae1d into master Jun 6, 2022
@iohk-bors iohk-bors bot deleted the kderme/hf-integration branch June 6, 2022 06:14
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.

3 participants