Make wallet run on Mary #2400

Anviking · 2020-12-16T19:28:13Z

Issue Number

ADP-623

Overview

Make the wallet run on Mary,
MA Value types are converted to/from TokenBundles.
Roundtrip tests for Value/TokenBundle
Fix disabled era error https://input-output-rnd.slack.com/archives/CFKLUH4R0/p1608146419455500
Test cluster does hard forks at start up - instantly.
Uses latest cardano-node master branch, with Add SerialiseAsRawBytes instances for PolicyId and AssetName IntersectMBO/cardano-node#2270 merged.

Comments

I imagine properly thinking about and removing the duplication would be pretty fun. (rvl: we can do that in the next PR)
Local test cluster changes for testing in multiple eras ⇒ Make hard fork era configurable in local test cluster #2438

rvl · 2020-12-17T12:45:03Z

I pushed some more WIP:

Added a stack snapshot for Update dependencies IntersectMBO/cardano-node#2216.
Adjusted cluster launch code and config to use InstaFork™ technology.
Renamed "shelley test cluster" to just "local cluster".
Removed the commits from Fix bug preventing txs to be listed when filter spans multiple eras #2397 and Tweak genesis params to make integration tests almost twice as fast #2391 and rebased on master.

KtorZ · 2021-01-04T13:06:25Z

@Anviking why was it necessary to re-generate all the faucet keys 🤔 ?

Anviking · 2021-01-04T13:07:32Z

@KtorZ I think @rvl renamed shelley-test-cluster to local-cluster, including the folder the keys where in

rvl · 2021-01-05T06:58:02Z

I pulled out the local-cluster rename commits. We can do that another time.

jonathanknowles · 2021-01-08T11:16:29Z

lib/core/src/Cardano/Wallet/Primitive/Types/TokenPolicy.hs

@@ -5,10 +5,10 @@
 module Cardano.Wallet.Primitive.Types.TokenPolicy
    (
      -- * Token Policies
-      TokenPolicyId
+      TokenPolicyId (..)


@Anviking

Just a thought.

Rather than exporting the default data constructors for TokenPolicyId and TokenName, perhaps we could define and export smart constructors instead?

It would be my preference to not export these default constructors, as it opens the door to people accidentally creating invalid token policy IDs (which should have a certain length), and this could cause subtle regressions in other places.

In the case of TokenName, there is actually already a FromText instance. Right now, this never fails, but if token names were to require validation, then calling fromText would be safer than using the default constructor.

It would be my preference to not export these default constructors, as it opens the door to people accidentally

👍 Good catch, agree.

Though, I wonder what the best solution is 🤔

were to require validation, then calling fromText would be safer than using the default constructor.

If fromText fails in the node-to-wallet type-conversion, then the wallet and node have different validation rules, and surely the node's must be the correct ones. Do we actually want to validate in the conversion?

I exported those constructors, because it's impractical not to have them. PolicyId and AssetName values coming from the node are already validated.

I exported those constructors, because it's impractical not to have them. PolicyId and AssetName values coming from the node are already validated.

Well, one alternative is to create functions unsafePolicyId and unsafePolicyName and export these instead of the default data constructors.

Advantages:

Callers of these functions are explicitly reminded that they're using unsafe functions. The onus is on them to check that the input is valid.

When people reviewing code see a new call to unsafeX, they can ask the question: "Has this input been validated?"

We can easily track usages of unsafe functions with a simple call to grep (or ag). This is a bit harder to do with the default data constructor.

The type constructor itself could be called UnsafePolicyId if you like.

@jonathanknowles I think we do not care much about that last bit 😊 ... if that really bothers you: https://hackage.haskell.org/package/quiet-0.2/docs/Quiet.html

@jonathanknowles I think we do not care much about that last bit blush ...

Fair enough.

if that really bothers you: https://hackage.haskell.org/package/quiet-0.2/docs/Quiet.html

We actually are already using Quiet here:

newtype TokenPolicyId = TokenPolicyId { unTokenPolicyId :: Hash "TokenPolicy" } deriving stock (Eq, Ord, Generic) deriving (Read, Show) via (Quiet TokenPolicyId)

I would have been slightly in favour of unsafePolicyId over UnsafePolicyId for the sake of:

Less confusing Show output

No unintended de-construction outside the module

but won't object, either.

@Anviking we also need de-construction in Compatibility.hs.

The situation is a bit ridiculous with the multiple layers of wrapping around a 32 byte bytestring (maximum length not enforced by our code by the way). And Text is probably not the correct inner type anyway!

Ok, fair, perhaps.

Co-authored-by: Johannes Lund <[email protected]> Co-authored-by: Rodney Lorrimar <[email protected]>

It's not pretty, sorry.

rvl · 2021-01-12T05:45:21Z

bors try

Anviking

lgtm

Anviking · 2021-01-12T09:23:40Z

lib/shelley/src/Cardano/Wallet/Shelley/Network.hs

+                return $ fromPoolDistr <$> result
+            AnyCardanoEra ByronEra ->
+                -- see also: #2419
+                throwE $ ErrStakeDistributionQuery "Can't query stake distribution in byron era"


Anviking · 2021-01-12T09:57:11Z

Seems both buildkite and hydra passed in the bors try, but the bors status has failed to be updated
bors r+

2400: Make wallet run on Mary r=Anviking a=Anviking # Issue Number ADP-623 # Overview - [x] Make the wallet run on Mary, - [x] MA Value types are converted to/from TokenBundles. - [x] Roundtrip tests for Value/TokenBundle - [x] Fix disabled era error https://input-output-rnd.slack.com/archives/CFKLUH4R0/p1608146419455500 - [x] Test cluster does hard forks at start up - instantly. - [x] Uses latest cardano-node master branch, with IntersectMBO/cardano-node#2270 merged. # Comments - I imagine properly thinking about and removing the duplication would be pretty fun. (rvl: we can do that in the next PR) - Local test cluster changes for testing in multiple eras ⇒ #2438 Co-authored-by: Rodney Lorrimar <[email protected]> Co-authored-by: Johannes Lund <[email protected]>

iohk-bors · 2021-01-12T10:12:15Z

try

Timed out.

Nix eval error

iohk-bors · 2021-01-12T10:57:31Z

Build failed:

buildkite/cardano-wallet

src/Test/Integration/Scenario/API/Shelley/StakePools.hs:576:5:
--
  | 1) API Specifications, SHELLEY_STAKE_POOLS, STAKE_POOLS_JOIN_04 - Rewards accumulate
  | uncaught exception: RequestException
  | DecodeFailure "{\"code\":\"network_query_failed\",\"message\":\"Unable to query the ledger at the moment. This error has been logged. Trying again in a bit might work.\"}"
  |  
  | To rerun use: --match "/API Specifications/SHELLEY_STAKE_POOLS/STAKE_POOLS_JOIN_04 - Rewards accumulate/"
  |  
  | src/Test/Integration/Scenario/CLI/Shelley/HWWallets.hs:153:26:
  | 2) CLI Specifications, SHELLEY_CLI_HW_WALLETS, HW_WALLETS_01x - Restoration from account public key preserves funds
  | While verifying ApiWallet {id = ApiT {getApiT = WalletId {getWalletId = 42245c2f417e6f1a62a2b3a7e501d544320d59d5}}, addressPoolGap = ApiT {getApiT = AddressPoolGap {getAddressPoolGap = 20}}, balance = ApiT {getApiT = WalletBalance {available = Quantity {getQuantity = 0}, total = Quantity {getQuantity = 0}, reward = Quantity {getQuantity = 0}}}, delegation = ApiWalletDelegation {active = ApiWalletDelegationNext {status = NotDelegating, target = Nothing, changesAt = Nothing}, next = []}, name = ApiT {getApiT = WalletName {getWalletName = "Wallet from pub key"}}, passphrase = Nothing, state = ApiT {getApiT = Syncing (Quantity {getQuantity = Percentage {getPercentage = 7569 % 10000}})}, tip = ApiBlockReference {absoluteSlotNumber = ApiT {getApiT = SlotNo 4037}, slotId = ApiSlotId {epochNumber = ApiT {getApiT = EpochNo {unEpochNo = 80}}, slotNumber = ApiT {getApiT = SlotInEpoch {unSlotInEpoch = 37}}}, time = 2021-01-12 10:48:58.4 UTC, block = ApiBlockInfo {height = Quantity {getQuantity = 2000}}}}
  | Waited longer than 90s to resolve action: "Wallet balance is as expected on wallet from pubKey".
  | expected: Quantity {getQuantity = 1000000}
  | but got: Quantity {getQuantity = 0}
  |  
  | To rerun use: --match "/CLI Specifications/SHELLEY_CLI_HW_WALLETS/HW_WALLETS_01x - Restoration from account public key preserves funds/"

#2320

#2428

Anviking · 2021-01-12T11:00:29Z

bors r+

2400: Make wallet run on Mary r=Anviking a=Anviking # Issue Number ADP-623 # Overview - [x] Make the wallet run on Mary, - [x] MA Value types are converted to/from TokenBundles. - [x] Roundtrip tests for Value/TokenBundle - [x] Fix disabled era error https://input-output-rnd.slack.com/archives/CFKLUH4R0/p1608146419455500 - [x] Test cluster does hard forks at start up - instantly. - [x] Uses latest cardano-node master branch, with IntersectMBO/cardano-node#2270 merged. # Comments - I imagine properly thinking about and removing the duplication would be pretty fun. (rvl: we can do that in the next PR) - Local test cluster changes for testing in multiple eras ⇒ #2438 Co-authored-by: Rodney Lorrimar <[email protected]> Co-authored-by: Johannes Lund <[email protected]>

iohk-bors · 2021-01-12T15:03:26Z

Timed out.

#nix eval error

Removing this seems to fix the nix build on my old mac. Hopefully this fixes CI too. I'd guess the fix now already exists somewhere upstream.

Anviking · 2021-01-12T18:13:18Z

🤞
bors r+

2400: Make wallet run on Mary r=Anviking a=Anviking # Issue Number ADP-623 # Overview - [x] Make the wallet run on Mary, - [x] MA Value types are converted to/from TokenBundles. - [x] Roundtrip tests for Value/TokenBundle - [x] Fix disabled era error https://input-output-rnd.slack.com/archives/CFKLUH4R0/p1608146419455500 - [x] Test cluster does hard forks at start up - instantly. - [x] Uses latest cardano-node master branch, with IntersectMBO/cardano-node#2270 merged. # Comments - I imagine properly thinking about and removing the duplication would be pretty fun. (rvl: we can do that in the next PR) - Local test cluster changes for testing in multiple eras ⇒ #2438 Co-authored-by: Rodney Lorrimar <[email protected]> Co-authored-by: Johannes Lund <[email protected]>

iohk-bors · 2021-01-12T19:24:14Z

Build failed:

ci/hydra-build:required

Failures:

  test/unit/Cardano/Wallet/DB/StateMachine.hs:1333:28:
  1) Cardano.Wallet.DB.Sqlite, Validate generators & shrinkers, Shrinker for CreateWallet
       Timed out.

  To rerun use: --match "/Cardano.Wallet.DB.Sqlite/Validate generators & shrinkers/Shrinker for CreateWallet/"

Randomized with seed 2076883091

#2393

Anviking · 2021-01-12T19:48:00Z

bors r+

iohk-bors · 2021-01-12T20:26:16Z

Build succeeded:

rvl changed the title ~~WIP: Very course Mary-integration~~ WIP: Very coarse Mary-integration Dec 17, 2020

rvl force-pushed the anviking/ADP-623/mary branch from 362deec to 3daae77 Compare December 17, 2020 12:40

rvl force-pushed the anviking/ADP-623/mary branch 4 times, most recently from 2052ac9 to 6fd0966 Compare December 23, 2020 16:26

KtorZ assigned Anviking Dec 31, 2020

KtorZ added the ADDING FEATURE Mark a PR as adding a new feature, for auto-generated CHANGELOG label Dec 31, 2020

rvl force-pushed the anviking/ADP-623/mary branch 2 times, most recently from 8464693 to 8c6cf68 Compare January 4, 2021 08:43

rvl force-pushed the anviking/ADP-623/mary branch 2 times, most recently from 8e1811f to ec663f1 Compare January 5, 2021 06:56

Anviking force-pushed the anviking/ADP-623/mary branch from 32c5d03 to eb433f7 Compare January 5, 2021 14:24

Anviking changed the title ~~WIP: Very coarse Mary-integration~~ Make wallet run on Mary (without native token support) Jan 5, 2021

Anviking force-pushed the anviking/ADP-623/mary branch 5 times, most recently from 9ec04e9 to 43b357c Compare January 7, 2021 12:28

rvl force-pushed the anviking/ADP-623/mary branch from 975ebde to 84f250b Compare January 7, 2021 12:41

rvl mentioned this pull request Jan 8, 2021

Flaky: ProcessHasExited "cluster didn't start correctly #2427

Closed

rvl linked an issue Jan 8, 2021 that may be closed by this pull request

Flaky: ProcessHasExited "cluster didn't start correctly #2427

Closed

rvl force-pushed the anviking/ADP-623/mary branch 2 times, most recently from b736706 to a6b8874 Compare January 8, 2021 10:35

jonathanknowles reviewed Jan 8, 2021

View reviewed changes

rvl force-pushed the anviking/ADP-623/mary branch from b0c4cbd to 991cc68 Compare January 9, 2021 08:34

rvl and others added 9 commits January 12, 2021 11:48

Faster test cluster startup with insta-fork

925b4d9

Add basic Mary support

fc3d8c5

Co-authored-by: Johannes Lund <[email protected]> Co-authored-by: Rodney Lorrimar <[email protected]>

Add cardanoCliEra helper

42b85fa

fixup: add missing cases to ensure allegra still works

0816e86

Move shelley import

cff8480

MetaData -> Metadata

ea84bd8

Add decodeSignedTx for Mary

d17c3e4

It's not pretty, sorry.

Export TokenPolicyId and TokenName constructors

68454b0

Add conversions between Cardano.Api.Value and TokenBundle

83ecc78

rvl force-pushed the anviking/ADP-623/mary branch from ec3df0f to 83ecc78 Compare January 12, 2021 03:52

rvl mentioned this pull request Jan 12, 2021

Make hard fork era configurable in local test cluster #2438

Merged

iohk-bors bot added a commit that referenced this pull request Jan 12, 2021

Try #2400:

7605277

KtorZ approved these changes Jan 12, 2021

View reviewed changes

Anviking commented Jan 12, 2021

View reviewed changes

Remove seemingly redundant nix fix

d818cb7

Removing this seems to fix the nix build on my old mac. Hopefully this fixes CI too. I'd guess the fix now already exists somewhere upstream.

iohk-bors bot merged commit f7fdc3c into master Jan 12, 2021

iohk-bors bot deleted the anviking/ADP-623/mary branch January 12, 2021 20:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make wallet run on Mary #2400

Make wallet run on Mary #2400

Anviking commented Dec 16, 2020 •

edited by rvl

Loading

rvl commented Dec 17, 2020

KtorZ commented Jan 4, 2021

Anviking commented Jan 4, 2021

rvl commented Jan 5, 2021

jonathanknowles Jan 8, 2021 •

edited

Loading

Anviking Jan 8, 2021

rvl Jan 8, 2021

jonathanknowles Jan 9, 2021 •

edited

Loading

rvl Jan 9, 2021

KtorZ Jan 11, 2021

jonathanknowles Jan 11, 2021

Anviking Jan 11, 2021 •

edited

Loading

rvl Jan 12, 2021

Anviking Jan 12, 2021

rvl commented Jan 12, 2021

Anviking left a comment

Anviking Jan 12, 2021

Anviking commented Jan 12, 2021

iohk-bors bot commented Jan 12, 2021 •

edited by Anviking

Loading

iohk-bors bot commented Jan 12, 2021 •

edited by Anviking

Loading

Anviking commented Jan 12, 2021

iohk-bors bot commented Jan 12, 2021 •

edited by Anviking

Loading

Anviking commented Jan 12, 2021

iohk-bors bot commented Jan 12, 2021 •

edited by Anviking

Loading

Anviking commented Jan 12, 2021

iohk-bors bot commented Jan 12, 2021

Make wallet run on Mary #2400

Make wallet run on Mary #2400

Conversation

Anviking commented Dec 16, 2020 • edited by rvl Loading

Issue Number

Overview

Comments

rvl commented Dec 17, 2020

KtorZ commented Jan 4, 2021

Anviking commented Jan 4, 2021

rvl commented Jan 5, 2021

jonathanknowles Jan 8, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jonathanknowles Jan 9, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Anviking Jan 11, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rvl commented Jan 12, 2021

Anviking left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Anviking commented Jan 12, 2021

iohk-bors bot commented Jan 12, 2021 • edited by Anviking Loading

try

iohk-bors bot commented Jan 12, 2021 • edited by Anviking Loading

Anviking commented Jan 12, 2021

iohk-bors bot commented Jan 12, 2021 • edited by Anviking Loading

Anviking commented Jan 12, 2021

iohk-bors bot commented Jan 12, 2021 • edited by Anviking Loading

Anviking commented Jan 12, 2021

iohk-bors bot commented Jan 12, 2021

Anviking commented Dec 16, 2020 •

edited by rvl

Loading

jonathanknowles Jan 8, 2021 •

edited

Loading

jonathanknowles Jan 9, 2021 •

edited

Loading

Anviking Jan 11, 2021 •

edited

Loading

iohk-bors bot commented Jan 12, 2021 •

edited by Anviking

Loading

iohk-bors bot commented Jan 12, 2021 •

edited by Anviking

Loading

iohk-bors bot commented Jan 12, 2021 •

edited by Anviking

Loading

iohk-bors bot commented Jan 12, 2021 •

edited by Anviking

Loading