Skip to content
This repository has been archived by the owner on Aug 18, 2020. It is now read-only.

[release/1.3.1] [CO-354] Implement NetworkMagic logic #3561

Merged
merged 2 commits into from
Sep 18, 2018

Conversation

mhuesch
Copy link
Contributor

@mhuesch mhuesch commented Sep 7, 2018

Description

This PR/commit implements part (4) of our plan.

We pipe the full NetworkMagic to the "frontier" outlined above. This means bridging the gap from where we have ProtocolMagic to where we need NetworkMagic (and had NMMustBeNothing hardcoded).

Since PR #3559 splits the test-suites, we see most of the changes in this PR are to source-code files.

Linked issue

https://iohk.myjetbrains.com/youtrack/issue/CO-354

Type of change

  • [~] 🐞 Bug fix (non-breaking change which fixes an issue)
  • 🛠 New feature (non-breaking change which adds functionality)
  • ⚠️ Breaking change (fix or feature that would cause existing functionality to change)
  • [~] 🏭 Refactoring that does not change existing functionality but does improve things like code readability, structure etc
  • [~] 🔨 New or improved tests for existing code
  • [~] ⛑ git-flow chore (backport, hotfix, etc)
    ^ this PR adds the capability for differently formatted Addresses, which include a NetworkMagic value. Thus it is a breaking change for testnets (NMJust), but backwards compatible for mainnet/staging (NMNothing).

Developer checklist

  • I have read the style guide document, and my code follows the code style of this project.
  • If my code deals with exceptions, it follows the guidelines.
  • I have updated any documentation accordingly, if needed. Documentation changes can be reflected in opening a PR on cardanodocs.com, amending the inline Haddock comments, any relevant README file or one of the document listed in the docs directory.
  • [~] CHANGELOG entry has been added and is linked to the correct PR on GitHub.
    ^ there is no 1.3.1 section in the CHANGELOG, and this is dev work on a release branch, so I'm skipping this.

Testing checklist

@mhuesch
Copy link
Contributor Author

mhuesch commented Sep 7, 2018

N.B. this PR contains commits which are part of PR #3556, PR #3558, and PR #3559. Thus, the "Files Changed" is inflated, and if someone reviews this before #3556 / #3558 / #3559 are merged, it would be simplest to review only the commit:

f3f87824f9 [CO-354] Implement `NetworkMagic` logic

(hashes might change if we force-push)

EDIT:
I, @intricate, have rebased this branch on release/1.3.1 after the merging of #3556, #3558, and #3559 so this comment no longer applies.

@mhuesch mhuesch force-pushed the intricate+mhuesch/CO-354/implement-networkmagic branch from f3f8782 to bd9a5d5 Compare September 10, 2018 19:18
@mhuesch mhuesch force-pushed the intricate+mhuesch/CO-354/implement-networkmagic branch from bd9a5d5 to bc381f1 Compare September 10, 2018 21:08
@mhuesch mhuesch changed the title [CO-354] Implement NetworkMagic logic [release/1.3.1] [CO-354] Implement NetworkMagic logic Sep 10, 2018
@mhuesch mhuesch force-pushed the intricate+mhuesch/CO-354/implement-networkmagic branch from bc381f1 to 10ec9fa Compare September 12, 2018 08:05
@intricate intricate force-pushed the intricate+mhuesch/CO-354/implement-networkmagic branch from 10ec9fa to 86386e2 Compare September 13, 2018 20:29
erikd
erikd previously approved these changes Sep 13, 2018
Copy link
Member

@erikd erikd left a comment

Choose a reason for hiding this comment

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

LGTM! Looks like a lot of very tedious work to get to this stage.

Copy link
Contributor

@disassembler disassembler left a comment

Choose a reason for hiding this comment

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

You need to change the asset locked wallet and address in wallet integration test:

$ git grep Ae2tdPwUPEZ5YjF9WuDoWfCZLPQ56MdQC6CZa2VKwMVRVqBBfTLPNcPvET4
wallet-new/integration/Util.hs:    WalletId "Ae2tdPwUPEZ5YjF9WuDoWfCZLPQ56MdQC6CZa2VKwMVRVqBBfTLPNcPvET4"

$ git grep DdzFFzCqrhswMWoTiWaqXUDZJuYUx63qB6Aq8rbVbhFbc8NWqhpZkC7Lhn5eVA7kWf4JwKvJ9PqQF78AewMCzDZLabkzm99rFzpNDKp5
scripts/test/wallet/integration/default.nix:    assetLockAddresses = [ "DdzFFzCqrhswMWoTiWaqXUDZJuYUx63qB6Aq8rbVbhFbc8NWqhpZkC7Lhn5eVA7kWf4JwKvJ9PqQF78AewMCzDZLabkzm9>

You'll need to spin up a demo cluster with this PR, grab any of the genesis wallets that are imported (e.g. "2cWKMJemoBakxGHgy85BN6JJooy54EmWxCoobAXe3NRfyEbuMnJAZSr5oAzD5F5aoodRD") and find the address in that wallet. Then update both of the above files with the wallet Id and the blacklisted address in that wallet.

mhuesch and others added 2 commits September 18, 2018 08:47
Merged with:

WIP: wallet and wallet-new build
[CO-354] Fix build errors in `tools`
[CO-354] Fix `core` testsuite
    Addresss with `NMJust <word8>` in them are now 4 bytes longer than
    their `NMNothing` counterparts (which stayed the same length as
    before, for backwards compatibility reasons).

    I (mhueschen) am not sure why they grew by 4 bytes, rather than by
    1 or 2.
[CO-354] Fix `wallet` testsuite - it now passes.
    Parameterize configuration properly, by ProtocolMagic, so that address
    generation and verification are consistent.
[CO-354] WIP: Fix some of the `generator` tests
    A few remain, complaining about richmen.
[CO-354] Fix wallet test-suite build error
[CO-354] Clean up wallet `AddressSpec`
[CO-354] Split `core` tests (NMNothing/Just)
[CO-354] Split `wallet-new` tests (NMNothing/Just)
[CO-354] Split `txp` tests (NMNothing/Just)
[CO-354] Split `lib` tests (NMNothing/Just)
[CO-354] Split `client` tests (NMNothing/Just)
[CO-354] Split `explorer` tests (NMNothing/Just)
[CO-354] Split `generator` tests (NMNothing/Just)
[CO-354] Split `wallet` tests (NMNothing/Just)
[CO-354] Fix the `LrcSpec` issue in the `generator` test-suite

Co-authored-by: Michael Hueschen <[email protected]>
Co-authored-by: Luke Nadur <[email protected]>
@intricate intricate force-pushed the intricate+mhuesch/CO-354/implement-networkmagic branch from d9f1167 to a2d569a Compare September 18, 2018 13:48
@erikd erikd merged commit 435cd3f into release/1.3.1 Sep 18, 2018
@erikd erikd deleted the intricate+mhuesch/CO-354/implement-networkmagic branch September 18, 2018 20:58
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants