Byron-Rewrite node's integration #1303

KtorZ · 2020-01-24T00:44:04Z

Issue Number

Overview

efe7fcd
Initial NetworkLayer implementation for Byron
53b074e
rename 'networkTip' to 'currentNodeTip' in NetworkLayer
This name was misleading. What 'networkTip' actually returns is the tip of the local
chain of the underlying node. The network tip can be known statically from the blockchain
start date.
6f2aee6
remove 'findIntersection' from top-level NetworkLayer interface
And move it only to tests, where it's used
0d0a62e
make 'send' timeout when no response is given in a timely manner
This avoids blocking potentially indefinitely if the Network Client
somehow dies at the other end and the connection isn't cleaned up
correctly.
baa4106
Support multiple node connections through the network layer
fbab14e
add note about retrying in the chain follower
574c3a5
remove unused 'TxWitness' data-type
67028cd
re-implement base of transaction layer for Byron
fa55bab
remove unused 'dummyAddress' and 'dummyKey'
1ae2759
disable shelley-specific endpoint in Byron server
Until the address format is stabilized on the Haskell side, we will keep this one off
5bd164a
re-generate nix machinery

Comments

@Anviking So, this is mostly "complete" now. We will need to re-introduce the transaction submission in the legacy API (potentially?) and also, enable / resurrect integration tests and actually all other tests regarding the byron stuff. Will do in some other PR(s).

I've cleaned up a few things along the way, some remainders of the http-bridge era.

Anviking · 2020-01-24T10:36:20Z

lib/byron/src/Cardano/Wallet/Byron/Network.hs

+            Left e  -> throw e
+            Right intersection -> pure $ Cursor intersection
+
+    _nextBlocks _ = do


The _ would mean:

nextBlocks t >> nextBlocks t doesn't work as you'd expect

RetryImmediately :: FollowAction will silently not work as expected when passed to follow

🤷‍♂

I am not sure to follow you on this ?

Ah, yes. The protocol is stateful now indeed. So using the same cursor twice will have an unexpected behavior. I'll see if we can handle that properly.

Added a note.

It feels much better to me now when you put the queue inside the Cursor 👍

One can look at the cursor, see that there is some kind of state there, and not be surprised.

Also, initCursor will always create a new node-connection & cursor. This is neat. Had the state been in the NetworkLayer, we could have accidentally messed up an ongoing chain-sync by calling initCursor in the middle of it (which would have changed the node's notion of where the wallet's tip). I imagine at least.

lib/core/src/Cardano/Wallet/Network.hs

KtorZ · 2020-01-24T17:43:58Z

bors try

iohk-bors · 2020-01-24T17:58:53Z

try

Build failed

buildkite/cardano-wallet

Anviking · 2020-01-29T10:32:20Z

bors try

iohk-bors · 2020-01-29T10:40:03Z

try

Build failed

buildkite/cardano-wallet

This name was misleading. What 'networkTip' actually returns is the tip of the local chain of the underlying node. The network tip can be known statically from the blockchain start date.

And move it only to tests, where it's used

This avoids blocking potentially indefinitely if the Network Client somehow dies at the other end and the connection isn't cleaned up correctly.

Until the address format is stabilized on the Haskell side, we will keep this one off

Cabal v2-build was failing with: Resolving dependencies... cabal: Could not resolve dependencies: [__0] trying: cardano-wallet-byron-2020.1.21 (user goal) [__1] unknown package: typed-protocols-cbor (dependency of cardano-wallet-byron) [__1] fail (backjumping, conflict set: cardano-wallet-byron, typed-protocols-cbor) After searching the rest of the dependency tree exhaustively, these were the goals I've had most trouble fulfilling: cardano-wallet-byron, typed-protocols-cbor This dependency needs to be added to cabal.project. However it's easier to use stack.yaml as the single source of truth rather than keeping both files in sync. Cabal v2-configure can be run inside nix-shell, which will provide to Cabal the exact dependencies defined by stack.yaml.

@Anviking

1303: Byron-Rewrite node's integration r=Anviking a=KtorZ # Issue Number  # Overview  - efe7fcd Initial NetworkLayer implementation for Byron - 53b074e rename 'networkTip' to 'currentNodeTip' in NetworkLayer This name was misleading. What 'networkTip' actually returns is the tip of the local chain of the underlying node. The network tip can be known statically from the blockchain start date. - 6f2aee6 remove 'findIntersection' from top-level NetworkLayer interface And move it only to tests, where it's used - 0d0a62e make 'send' timeout when no response is given in a timely manner This avoids blocking potentially indefinitely if the Network Client somehow dies at the other end and the connection isn't cleaned up correctly. - baa4106 Support multiple node connections through the network layer - fbab14e add note about retrying in the chain follower - 574c3a5 remove unused 'TxWitness' data-type - 67028cd re-implement base of transaction layer for Byron - fa55bab remove unused 'dummyAddress' and 'dummyKey' - 1ae2759 disable shelley-specific endpoint in Byron server Until the address format is stabilized on the Haskell side, we will keep this one off - 5bd164a re-generate nix machinery # Comments  @Anviking So, this is mostly "complete" now. We will need to re-introduce the transaction submission in the legacy API (potentially?) and also, enable / resurrect integration tests and actually all other tests regarding the byron stuff. Will do in some other PR(s). I've cleaned up a few things along the way, some remainders of the http-bridge era.  Co-authored-by: KtorZ <[email protected]> Co-authored-by: Rodney Lorrimar <[email protected]> Co-authored-by: Johannes Lund <[email protected]>

iohk-bors · 2020-01-29T14:40:10Z

Build failed

buildkite/cardano-wallet

src/Test/Integration/Framework/DSL.hs:319:16:
--
  | 1) API Specifications TRANS_CREATE_02 - Multiple Output Tx to single wallet
  | expected: Quantity {getQuantity = 800000000000}
  | but got: Quantity {getQuantity = 999999999966}
  |  
  | To rerun use: --match "/API Specifications/TRANS_CREATE_02 - Multiple Output Tx to single wallet/"

Randomized with seed 339388683

Anviking · 2020-01-29T14:44:04Z

I was not able to reproduce locally. I thought this was a flaky test. But it is here failing reliably 🤔

Anviking · 2020-01-29T18:55:49Z

bors try

iohk-bors · 2020-01-29T19:39:56Z

try

Build failed

buildkite/cardano-wallet

  src/Test/Integration/Framework/DSL.hs:319:16: 
  1) API Specifications TRANS_DELETE_01 - Shelley: Can forget pending transaction
       expected: Quantity {getQuantity = 1000000000000}
        but got: Quantity {getQuantity = 999999999976}

  To rerun use: --match "/API Specifications/TRANS_DELETE_01 - Shelley: Can forget pending transaction/"

Randomized with seed 833445295

To get more info on integration failures

Anviking · 2020-01-29T20:34:51Z

bors try

iohk-bors · 2020-01-29T21:05:15Z

try

Build succeeded

buildkite/cardano-wallet

Anviking · 2020-01-29T21:06:42Z

bors try

iohk-bors · 2020-01-29T21:45:06Z

try

Build succeeded

buildkite/cardano-wallet

Anviking · 2020-01-29T21:53:41Z

bors r+

@Anviking

1303: Byron-Rewrite node's integration r=Anviking a=KtorZ # Issue Number  # Overview  - efe7fcd Initial NetworkLayer implementation for Byron - 53b074e rename 'networkTip' to 'currentNodeTip' in NetworkLayer This name was misleading. What 'networkTip' actually returns is the tip of the local chain of the underlying node. The network tip can be known statically from the blockchain start date. - 6f2aee6 remove 'findIntersection' from top-level NetworkLayer interface And move it only to tests, where it's used - 0d0a62e make 'send' timeout when no response is given in a timely manner This avoids blocking potentially indefinitely if the Network Client somehow dies at the other end and the connection isn't cleaned up correctly. - baa4106 Support multiple node connections through the network layer - fbab14e add note about retrying in the chain follower - 574c3a5 remove unused 'TxWitness' data-type - 67028cd re-implement base of transaction layer for Byron - fa55bab remove unused 'dummyAddress' and 'dummyKey' - 1ae2759 disable shelley-specific endpoint in Byron server Until the address format is stabilized on the Haskell side, we will keep this one off - 5bd164a re-generate nix machinery # Comments  @Anviking So, this is mostly "complete" now. We will need to re-introduce the transaction submission in the legacy API (potentially?) and also, enable / resurrect integration tests and actually all other tests regarding the byron stuff. Will do in some other PR(s). I've cleaned up a few things along the way, some remainders of the http-bridge era.  Co-authored-by: KtorZ <[email protected]> Co-authored-by: Rodney Lorrimar <[email protected]> Co-authored-by: Johannes Lund <[email protected]>

iohk-bors · 2020-01-29T22:32:00Z

Build succeeded

buildkite/cardano-wallet

The process to create these files is roughly: 1. run `stack2cabal` 2. work around cabal bugs (see haskell/cabal#5444) 3. disable tests to avoid build failures with --enable-tests from unrelated packages (cabal considers all packages defined via source-repository-package as *local* and will try to build its tests) 4. fix package constraints in cabal.project.freeze, where stackage constraings were overwritten in stack.yaml files (usually raising a version number) Rationale: 1. good to support all available tools as options for interested users 2. cabal allows easier managing of package bounds (and testing against latest package versiont) 3. cardano-foundation#1303 is an excellent workaround for nix users, but requires nix. This should be the go-to fallback in case cabal project files get out of sync for too long.

The process to create these files is roughly: 1. run `stack2cabal` 2. work around cabal bugs (see haskell/cabal#5444) 3. disable tests to avoid build failures with --enable-tests from unrelated packages (cabal considers all packages defined via source-repository-package as *local* and will try to build its tests) 4. fix package constraints in cabal.project.freeze, where stackage constraings were overwritten in stack.yaml files (usually raising a version number) Rationale: 1. good to support all available tools as options for interested users 2. cabal allows easier managing of package bounds (and testing against latest package versions) 3. cardano-foundation#1303 is an excellent workaround for nix users, but requires nix. This should be the go-to fallback in case cabal project files get out of sync for too long.

The process to create these files is roughly: 1. run `stack2cabal` 2. work around cabal bugs (see haskell/cabal#5444) with a script 3. disable tests to avoid build failures with --enable-tests from unrelated packages (cabal considers all packages defined via source-repository-package as *local* and will try to build its tests) 4. fix package constraints in cabal.project.freeze, where stackage constraings were overwritten in stack.yaml files (usually raising a version number) Rationale: 1. good to support all available tools as options for interested users 2. cabal allows easier managing of package bounds (and testing against latest package versions) 3. cardano-foundation#1303 is an excellent workaround for nix users, but requires nix. This should be the go-to fallback in case cabal project files get out of sync for too long.

2027: Allow to build with cabal r=rvl a=hasufell The process to create these files is roughly: 1. run `stack2cabal` 2. work around cabal bugs (see haskell/cabal#5444) 3. disable tests to avoid build failures with --enable-tests from unrelated packages (cabal considers all packages defined via source-repository-package as *local* and will try to build its tests) 4. fix package constraints in cabal.project.freeze, where stackage constraints were overwritten in stack.yaml files (usually raising a version number) Rationale: 1. good to support all available tools as options for interested users 2. cabal allows easier managing of package bounds (and testing against latest package versions) 3. #1303 is an excellent workaround for nix users, but requires nix. This should be the go-to fallback in case cabal project files get out of sync for too long. Co-authored-by: Julian Ospald <[email protected]> Co-authored-by: KtorZ <[email protected]> Co-authored-by: Rodney Lorrimar <[email protected]>

KtorZ requested a review from Anviking January 24, 2020 00:44

KtorZ self-assigned this Jan 24, 2020

Anviking reviewed Jan 24, 2020

View reviewed changes

KtorZ force-pushed the KtorZ/byron-rewrite-integration branch from b2f287f to b61ea6e Compare January 24, 2020 10:57

Anviking reviewed Jan 24, 2020

View reviewed changes

lib/core/src/Cardano/Wallet/Network.hs Show resolved Hide resolved

KtorZ force-pushed the KtorZ/byron-rewrite-integration branch 3 times, most recently from 3354c12 to 5bd164a Compare January 24, 2020 17:40

iohk-bors bot added a commit that referenced this pull request Jan 24, 2020

Try #1303:

530aee9

KtorZ marked this pull request as ready for review January 24, 2020 17:44

rvl mentioned this pull request Jan 29, 2020

Add shutdown handler for new launcher #1315

Merged

Anviking force-pushed the KtorZ/byron-rewrite-integration branch from 3e10d66 to d83459f Compare January 29, 2020 10:19

iohk-bors bot added a commit that referenced this pull request Jan 29, 2020

Try #1303:

e339eda

KtorZ and others added 13 commits January 29, 2020 12:01

Initial NetworkLayer implementation for Byron

772c056

rename 'networkTip' to 'currentNodeTip' in NetworkLayer

ad17cb1

This name was misleading. What 'networkTip' actually returns is the tip of the local chain of the underlying node. The network tip can be known statically from the blockchain start date.

remove 'findIntersection' from top-level NetworkLayer interface

443550d

And move it only to tests, where it's used

make 'send' timeout when no response is given in a timely manner

2fef24f

This avoids blocking potentially indefinitely if the Network Client somehow dies at the other end and the connection isn't cleaned up correctly.

Support multiple node connections through the network layer

4e7c1a0

add note about retrying in the chain follower

aa6f932

remove unused 'TxWitness' data-type

35862a0

re-implement base of transaction layer for Byron

8c4fe2c

remove unused 'dummyAddress' and 'dummyKey'

bbf2630

disable shelley-specific endpoint in Byron server

aa6e82c

Until the address format is stabilized on the Haskell side, we will keep this one off

re-generate nix machinery

9559649

Fix haddock build

7694177

Anviking mentioned this pull request Jan 29, 2020

[Failing Test Case]: TRANS_CREATE_02 #1284

Closed

iohk-bors bot added a commit that referenced this pull request Jan 29, 2020

Try #1303:

ca47304

Add HasCallStack to DSL expectation helpers

aaa0534

To get more info on integration failures

iohk-bors bot added a commit that referenced this pull request Jan 29, 2020

Try #1303:

850ac30

iohk-bors bot added a commit that referenced this pull request Jan 29, 2020

Try #1303:

87a042d

iohk-bors bot merged commit aaa0534 into master Jan 29, 2020

Anviking deleted the KtorZ/byron-rewrite-integration branch January 30, 2020 10:34

piotr-iohk added this to the (WB-53) Integrate cardano-wallet with Haskell Byron-rewrite milestone Jan 30, 2020

hasufell mentioned this pull request Aug 11, 2020

Allow to build with cabal #2027

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Byron-Rewrite node's integration #1303

Byron-Rewrite node's integration #1303

KtorZ commented Jan 24, 2020 •

edited

Loading

Anviking Jan 24, 2020

KtorZ Jan 24, 2020

KtorZ Jan 24, 2020

KtorZ Jan 24, 2020

Anviking Jan 28, 2020 •

edited

Loading

KtorZ commented Jan 24, 2020

iohk-bors bot commented Jan 24, 2020

Anviking commented Jan 29, 2020

iohk-bors bot commented Jan 29, 2020

iohk-bors bot commented Jan 29, 2020 •

edited by Anviking

Loading

Anviking commented Jan 29, 2020

Anviking commented Jan 29, 2020

iohk-bors bot commented Jan 29, 2020 •

edited by Anviking

Loading

Anviking commented Jan 29, 2020

iohk-bors bot commented Jan 29, 2020

Anviking commented Jan 29, 2020

iohk-bors bot commented Jan 29, 2020

Anviking commented Jan 29, 2020

iohk-bors bot commented Jan 29, 2020

Byron-Rewrite node's integration #1303

Byron-Rewrite node's integration #1303

Conversation

KtorZ commented Jan 24, 2020 • edited Loading

Issue Number

Overview

Comments

Anviking Jan 24, 2020

Choose a reason for hiding this comment

KtorZ Jan 24, 2020

Choose a reason for hiding this comment

KtorZ Jan 24, 2020

Choose a reason for hiding this comment

KtorZ Jan 24, 2020

Choose a reason for hiding this comment

Anviking Jan 28, 2020 • edited Loading

Choose a reason for hiding this comment

KtorZ commented Jan 24, 2020

iohk-bors bot commented Jan 24, 2020

try

Build failed

Anviking commented Jan 29, 2020

iohk-bors bot commented Jan 29, 2020

try

Build failed

iohk-bors bot commented Jan 29, 2020 • edited by Anviking Loading

Build failed

Anviking commented Jan 29, 2020

Anviking commented Jan 29, 2020

iohk-bors bot commented Jan 29, 2020 • edited by Anviking Loading

try

Build failed

Anviking commented Jan 29, 2020

iohk-bors bot commented Jan 29, 2020

try

Build succeeded

Anviking commented Jan 29, 2020

iohk-bors bot commented Jan 29, 2020

try

Build succeeded

Anviking commented Jan 29, 2020

iohk-bors bot commented Jan 29, 2020

Build succeeded

KtorZ commented Jan 24, 2020 •

edited

Loading

Anviking Jan 28, 2020 •

edited

Loading

iohk-bors bot commented Jan 29, 2020 •

edited by Anviking

Loading

iohk-bors bot commented Jan 29, 2020 •

edited by Anviking

Loading