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

Bump cardano-node to (almost) 1.28.0 #2755

Merged
merged 3 commits into from
Jul 14, 2021

Conversation

Anviking
Copy link
Member

@Anviking Anviking commented Jul 8, 2021

Issue Number

ADP-952

Overview

Comments

Basic Alonzo Era support is a non-goal for this PR. I implemented fromAlonzoTx and fromAlonzoBlock as error "unimplemented"s for now. Will fill them in in a follow-up PR, perhaps with integration cluster support at the same time.

@Anviking Anviking self-assigned this Jul 8, 2021
@Anviking Anviking force-pushed the anviking/ADP-952/cardano-node-1.28.0 branch from 71a9b8a to 42b592a Compare July 8, 2021 13:28
@@ -1191,36 +1207,42 @@ mkUnsignedTx era ttl cs md wdrls certs fees =
ShelleyBasedEraShelley -> toShelleyTxOut
ShelleyBasedEraAllegra -> toAllegraTxOut
ShelleyBasedEraMary -> toMaryTxOut
ShelleyBasedEraAlonzo -> error "todo: toAlonzoTxOut"
Copy link
Member Author

Choose a reason for hiding this comment

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

Suggested change
ShelleyBasedEraAlonzo -> error "todo: toAlonzoTxOut"
ShelleyBasedEraAlonzo -> error "todo(ADP-952): toAlonzoTxOut"

Copy link
Contributor

Choose a reason for hiding this comment

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

If you use something like this:

error "toAlonzoTxOut unimplemented" -- TODO: [ADP-952] toAlonzoTxOut

then it's easy to find all your TODOs with lentil:

$ lentil -t ADP-952 lib
263 source files [****************] 100%
lib/shelley/src/Cardano/Wallet/Shelley/Compatibility.hs
   521  fromAlonzoBlock [ADP-952]
   923  fromAlonzoTx [ADP-952]
  1148  datumHash [ADP-952]
  1167  datumHash [ADP-952]
  1186  datumHash [ADP-952]
 
lib/shelley/src/Cardano/Wallet/Shelley/Transaction.hs
  1173  We presumably need to provide the protocol params if our tx uses
        scripts? [ADP-952]
  1220  toAlonzoTxOut [ADP-952]

@Anviking
Copy link
Member Author

Anviking commented Jul 9, 2021

I now get

plutus-core/src/PlutusCore/Default/Builtins.hs:123:9: error:
--
  | • Could not deduce (GEq DefaultUni)
  | arising from a use of ‘makeBuiltinMeaning’
  | from the context: uni ~ DefaultUni
  | bound by the instance declaration
  | at plutus-core/src/PlutusCore/Default/Builtins.hs:120:10-60
  | or from: PlutusCore.Constant.Typed.HasConstantIn uni term
  | bound by the type signature for:
  | toBuiltinMeaning :: forall term.
  | PlutusCore.Constant.Typed.HasConstantIn uni term =>
  | DefaultFun
  | -> BuiltinMeaning term (CostingPart uni DefaultFun)
  | at plutus-core/src/PlutusCore/Default/Builtins.hs:122:5-20
  | • In the expression:
  | makeBuiltinMeaning
  | ((+) @Integer) (runCostingFunTwoArguments . paramAddInteger)
  | In an equation for ‘toBuiltinMeaning’:
  | toBuiltinMeaning AddInteger
  | = makeBuiltinMeaning
  | ((+) @Integer) (runCostingFunTwoArguments . paramAddInteger)
  | In the instance declaration for ‘ToBuiltinMeaning uni DefaultFun’
  | \|
  | 123 \|         makeBuiltinMeaning

for both stack and cabal, even though it seemed fixed locally (for stack build at least) 🤷‍♂️

@rvl rvl force-pushed the anviking/ADP-952/cardano-node-1.28.0 branch 4 times, most recently from 33cf5e1 to b7cbf51 Compare July 11, 2021 12:14
@rvl
Copy link
Contributor

rvl commented Jul 11, 2021

Thanks @Anviking - I have done a squash and rebase.

The stack build error on Buildkite was because the auto-generated nix files weren't up-to-date, and this was because it failed to push back to github for some weird reason. But anyway, I regenerated them.

Let's see what happens with the integration tests...

bors r+

@rvl rvl marked this pull request as ready for review July 11, 2021 12:16
iohk-bors bot added a commit that referenced this pull request Jul 11, 2021
2755: Bump cardano-node to (almost) 1.28.0 r=rvl a=Anviking

# Issue Number

ADP-952

# Overview

- [x] Bump to cardano-node master, in preparation for 1.28.0 release (input-output-hk/cardano-haskell#49)
- [x] Update readme 

# Comments

Basic Alonzo Era support is a non-goal for this PR. I implemented `fromAlonzoTx` and `fromAlonzoBlock` as `error "unimplemented"`s for now. Will fill them in in a follow-up PR, perhaps with integration cluster support at the same time.


- [Hydra jobset](https://hydra.iohk.io/jobset/Cardano/cardano-wallet-pr-2755#tabs-evaluations)
- [Cabal build](https://buildkite.com/input-output-hk/cardano-wallet-nightly/builds?branch=anviking%2FADP-952%2Fcardano-node-1.28.0)


Co-authored-by: Johannes Lund <[email protected]>
Co-authored-by: Rodney Lorrimar <[email protected]>
@iohk-bors
Copy link
Contributor

iohk-bors bot commented Jul 11, 2021

Build failed:

#expected - integration tests passed, unit tests failed because a second swagger change was needed.

@rvl rvl force-pushed the anviking/ADP-952/cardano-node-1.28.0 branch from b7cbf51 to a3bfdef Compare July 12, 2021 03:16
@rvl
Copy link
Contributor

rvl commented Jul 12, 2021

Again

bors r+

iohk-bors bot added a commit that referenced this pull request Jul 12, 2021
2755: Bump cardano-node to (almost) 1.28.0 r=rvl a=Anviking

# Issue Number

ADP-952

# Overview

- [x] Bump to cardano-node master, in preparation for 1.28.0 release (input-output-hk/cardano-haskell#49)
- [x] Update readme 

# Comments

Basic Alonzo Era support is a non-goal for this PR. I implemented `fromAlonzoTx` and `fromAlonzoBlock` as `error "unimplemented"`s for now. Will fill them in in a follow-up PR, perhaps with integration cluster support at the same time.


- [Hydra jobset](https://hydra.iohk.io/jobset/Cardano/cardano-wallet-pr-2755#tabs-evaluations)
- [Cabal build](https://buildkite.com/input-output-hk/cardano-wallet-nightly/builds?branch=anviking%2FADP-952%2Fcardano-node-1.28.0)


Co-authored-by: Johannes Lund <[email protected]>
Co-authored-by: Rodney Lorrimar <[email protected]>
@iohk-bors
Copy link
Contributor

iohk-bors bot commented Jul 12, 2021

Build failed:

#expected - Need to update golden json files, and "Time interpreter queried past the horizon" from integration tests.

@Anviking Anviking force-pushed the anviking/ADP-952/cardano-node-1.28.0 branch from 3ce1e23 to 266401b Compare July 13, 2021 10:34
@Anviking
Copy link
Member Author

bors try

iohk-bors bot added a commit that referenced this pull request Jul 13, 2021
@iohk-bors
Copy link
Contributor

iohk-bors bot commented Jul 13, 2021

try

Build succeeded:

@Anviking
Copy link
Member Author

The stack build error on Buildkite was because the auto-generated nix files weren't up-to-date, and this was because it failed to push back to github for some weird reason. But anyway, I regenerated them.

Many thanks!

Not sure if I should squash everything, but merging.
bors r+

iohk-bors bot added a commit that referenced this pull request Jul 13, 2021
2755: Bump cardano-node to (almost) 1.28.0 r=Anviking a=Anviking

# Issue Number

ADP-952

# Overview

- [x] Bump to cardano-node master, in preparation for 1.28.0 release (input-output-hk/cardano-haskell#49)
- [x] Update readme 

# Comments

Basic Alonzo Era support is a non-goal for this PR. I implemented `fromAlonzoTx` and `fromAlonzoBlock` as `error "unimplemented"`s for now. Will fill them in in a follow-up PR, perhaps with integration cluster support at the same time.


- [Hydra jobset](https://hydra.iohk.io/jobset/Cardano/cardano-wallet-pr-2755#tabs-evaluations)
- [Cabal build](https://buildkite.com/input-output-hk/cardano-wallet-nightly/builds?branch=anviking%2FADP-952%2Fcardano-node-1.28.0)


Co-authored-by: Johannes Lund <[email protected]>
Co-authored-by: Rodney Lorrimar <[email protected]>
@Anviking Anviking force-pushed the anviking/ADP-952/cardano-node-1.28.0 branch from 266401b to bd74724 Compare July 13, 2021 13:44
@iohk-bors
Copy link
Contributor

iohk-bors bot commented Jul 13, 2021

Canceled.

@Anviking Anviking force-pushed the anviking/ADP-952/cardano-node-1.28.0 branch from bd74724 to 86eecf2 Compare July 13, 2021 13:46
- Updates for cardano-node 1.28.0
- local-cluster: Add alonzo genesis
- Regenerate nix
- swagger: Add alonzo era to ApiNetworkParameters
- Api.Types: Update golden JSON files with Alonzo era info
- Bump iohk-nix to latest master branch
- Update readme

Co-Authored-By: Rodney Lorrimar <[email protected]>
@Anviking Anviking force-pushed the anviking/ADP-952/cardano-node-1.28.0 branch from 86eecf2 to e2a51ae Compare July 13, 2021 13:47
@Anviking
Copy link
Member Author

Fixed readme node revision and squashed

bors r+

iohk-bors bot added a commit that referenced this pull request Jul 13, 2021
2755: Bump cardano-node to (almost) 1.28.0 r=Anviking a=Anviking

# Issue Number

ADP-952

# Overview

- [x] Bump to cardano-node master, in preparation for 1.28.0 release (input-output-hk/cardano-haskell#49)
- [x] Update readme 

# Comments

Basic Alonzo Era support is a non-goal for this PR. I implemented `fromAlonzoTx` and `fromAlonzoBlock` as `error "unimplemented"`s for now. Will fill them in in a follow-up PR, perhaps with integration cluster support at the same time.


- [Hydra jobset](https://hydra.iohk.io/jobset/Cardano/cardano-wallet-pr-2755#tabs-evaluations)
- [Cabal build](https://buildkite.com/input-output-hk/cardano-wallet-nightly/builds?branch=anviking%2FADP-952%2Fcardano-node-1.28.0)


Co-authored-by: Johannes Lund <[email protected]>
@iohk-bors
Copy link
Contributor

iohk-bors bot commented Jul 13, 2021

Build failed:

/tmp/stack-97b0a942bf6b3b39/test-ghc-env: openBinaryFile: resource busy (file is locked)
--
  |  
  | /tmp/stack-97b0a942bf6b3b39/test-ghc-env: openBinaryFile: resource busy (file is locked)
  | error: Command exited with code 1!

#2695 - but with test-ghc-env instead of weeder, but still likely related to weeder. Annotating it as such...

@Anviking
Copy link
Member Author

bors r+

iohk-bors bot added a commit that referenced this pull request Jul 13, 2021
2755: Bump cardano-node to (almost) 1.28.0 r=Anviking a=Anviking

# Issue Number

ADP-952

# Overview

- [x] Bump to cardano-node master, in preparation for 1.28.0 release (input-output-hk/cardano-haskell#49)
- [x] Update readme 

# Comments

Basic Alonzo Era support is a non-goal for this PR. I implemented `fromAlonzoTx` and `fromAlonzoBlock` as `error "unimplemented"`s for now. Will fill them in in a follow-up PR, perhaps with integration cluster support at the same time.


- [Hydra jobset](https://hydra.iohk.io/jobset/Cardano/cardano-wallet-pr-2755#tabs-evaluations)
- [Cabal build](https://buildkite.com/input-output-hk/cardano-wallet-nightly/builds?branch=anviking%2FADP-952%2Fcardano-node-1.28.0)


Co-authored-by: Johannes Lund <[email protected]>
@Anviking
Copy link
Member Author

Anviking commented Jul 13, 2021

bors r-

Saw the PastHorizon failure again...

@iohk-bors
Copy link
Contributor

iohk-bors bot commented Jul 13, 2021

Canceled.

@Anviking
Copy link
Member Author

bors try

iohk-bors bot added a commit that referenced this pull request Jul 14, 2021
@Anviking
Copy link
Member Author

Anviking commented Jul 14, 2021

What is strange about https://hydra.iohk.io/build/6897950/nixlog/1 is that the PastHorizon failure come from reading a wallet in a part of a tests which doesn't use wallets. What actually caused the failure was CLI_NETWORK, which provided little details as of why it failed.

In https://buildkite.com/input-output-hk/cardano-wallet/builds/15481#6b969a8a-2a4a-4947-a21f-c1b41bae0882, there were PastHorizonErrors, but the retrying it ultimately made the test pass.

I can get PastHorizon errors locally using with stack test cardano-wallet:integration --ta '-j 25'.

I think there might be nothing in particular going on except that the tip-sync client had to compete with wallet chain-sync clients, and got out of date.

Things that might help down the line:

  1. Use one chain-sync client for multiple wallets to reduce load
  2. Chain-sync refactoring to handle rollbacks without re-negotiating (might reduce load)
  3. Add logic to prioritize tip-sync client above wallets
  4. Somehow get the TimeInterpreter from chain-sync, e.g. NodeToClient ChainSync with block metadata IntersectMBO/ouroboros-network#2901

Things we could do now:

  1. Do nothing and merge for now
  2. Reduce parallelism in CI and/or increase slot or epoch length
  3. Attempt to recover from PastHorizon errors by waiting for the interpreter to be such up to date that all Tip block we have seen (from all chain-sync threads) are within the horizon.
    • Obviously more tricky than the above, but might be doable 🤔

@iohk-bors
Copy link
Contributor

iohk-bors bot commented Jul 14, 2021

try

Build succeeded:

`readWalletMeta` has a hidden dependency on un up-to-date TimeInterpreter.
Under heavy load, on the local cluster with short horizons, it seems to
occasionally not be up to date.

The removed log message had Debug severity, so keeping it shouldn't be
that important.

Outstanding problems:
- Technically the pool follower also depends on an up-to-date
TimeInterpreter. Hopefully problems here are less likely than for the
pool worker.
- `readWalletMeta` should either never fail with PastHorizon errors, or
make the possibility clear to followers.
@Anviking
Copy link
Member Author

bors r+

iohk-bors bot added a commit that referenced this pull request Jul 14, 2021
2755: Bump cardano-node to (almost) 1.28.0 r=Anviking a=Anviking

# Issue Number

ADP-952

# Overview

- [x] Bump to cardano-node master, in preparation for 1.28.0 release (input-output-hk/cardano-haskell#49)
- [x] Update readme 

# Comments

Basic Alonzo Era support is a non-goal for this PR. I implemented `fromAlonzoTx` and `fromAlonzoBlock` as `error "unimplemented"`s for now. Will fill them in in a follow-up PR, perhaps with integration cluster support at the same time.


- [Hydra jobset](https://hydra.iohk.io/jobset/Cardano/cardano-wallet-pr-2755#tabs-evaluations)
- [Cabal build](https://buildkite.com/input-output-hk/cardano-wallet-nightly/builds?branch=anviking%2FADP-952%2Fcardano-node-1.28.0)


Co-authored-by: Johannes Lund <[email protected]>
@iohk-bors
Copy link
Contributor

iohk-bors bot commented Jul 14, 2021

Build failed:

[cardano-wallet.network:Error:47407] [2021-07-14 12:44:59.73 UTC] Time interpreter queried past the horizon. This should not happen because node tip is within safe-zone
Called from:
"src/Cardano/Wallet/Api/Server.hs:2941:15 in cardano-wallet-core-2021.6.11-5SxBQv4U3MgHTXQIA3DnMI:Cardano.Wallet.Api.Server"
Converting expression:
Some (ELet (ERelSlotToEpoch (EAbsToRelSlot (ELit (SlotNo 4411)))) (\x0 -> EPair (ERelToAbsEpoch (EVar x0)) (ESnd (EVar x0))))
 src/Test/Integration/Scenario/CLI/Network.hs:94:11:
  1) CLI Specifications, COMMON_CLI_NETWORK, CLI_NETWORK - cardano-wallet network information
       expected: ExitSuccess
        but got: ExitFailure 1

  To rerun use: --match "/CLI Specifications/COMMON_CLI_NETWORK/CLI_NETWORK - cardano-wallet network information/"

Randomized with seed 1647667095

Hopefully the likelihood of this failure has been reduced now, so perhaps can be labeled as:
#expected
although borderline sketchy to do so

@Anviking Anviking force-pushed the anviking/ADP-952/cardano-node-1.28.0 branch from 2355afb to fce4523 Compare July 14, 2021 13:45
@Anviking
Copy link
Member Author

bors r+

@iohk-bors
Copy link
Contributor

iohk-bors bot commented Jul 14, 2021

Build succeeded:

@iohk-bors iohk-bors bot merged commit d022113 into master Jul 14, 2021
@iohk-bors iohk-bors bot deleted the anviking/ADP-952/cardano-node-1.28.0 branch July 14, 2021 15:29
WilliamKingNoel-Bot pushed a commit that referenced this pull request Jul 14, 2021
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.

2 participants