Skip to content
This repository has been archived by the owner on Oct 10, 2023. It is now read-only.

Issue 1987/add stagenet #2000

Merged
merged 12 commits into from
Jan 5, 2022
Merged

Issue 1987/add stagenet #2000

merged 12 commits into from
Jan 5, 2022

Conversation

polaris-9r
Copy link
Collaborator

@polaris-9r polaris-9r commented Dec 29, 2021

Description

This ticket enables a user to switch to stagenet for THORChain, but leaves all other chains as chaosnet. This PR includes the following major changes

  • Updating @xchainjs libraries to new breaking change versions that add stagenet functionality for the base client class
  • Adding front-end selector functionality to select a Stagenet network, with coloring in red
  • Disable adding a ledger for a stagenet TC account, as it has not been enabled
  • Adding testing to validate stagenet network settings, plus new testing for isEnabledLedger check.

Notes

I was able to verify that we were able to create an sthor prefixed wallet addresses via a dev version of the electron app, and transfer sthor RUNE into that account.

Screenshots of new FE styling:

Chaosnet
image
image

Stagenet
image
image

NOTE: Leaving this in draft form at the moment since this is dependent on landing a change in xchainjs to add stagenet environment config: xchainjs/xchainjs-lib#456

TODO:

  • Land xchainjs changes to add stagenet
  • Uncomment changes that reference Network.Stagenet
  • Run through CI/CD to see if there are any other issues
  • Show pool information for mainnet for all non-TC assets, only stagenet for TC

#1987

Copy link
Collaborator

@veado veado left a comment

Choose a reason for hiding this comment

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

DRAFT LGTM.

Just two (small) things so far:
(1) Pls update helper toClientNetwork
(2) In case you want to change color of network label in header, here is the place to do

color: ${({ network }) => {
switch (network) {
case 'mainnet':
return palette('primary', 0)
case 'testnet':
return palette('warning', 0)
default:
return palette('text', 2)
}
}};

@polaris-9r polaris-9r marked this pull request as ready for review December 31, 2021 03:58
@polaris-9r
Copy link
Collaborator Author

@veado Wanted your perspective on one last part of this PR. So as you can see from the screenshots in my first post, the pool values are different between chaosnet and stagenet because they are hitting the mainnet vs stagenet versions of midgard. I can confirm though that in the wallet tab that the non-TC chain wallet addresses are the same values. Question is:

  • Do we want the pools tab to reflect all mainnet pool values? All stagenet pool values? (i.e. simply setting the env. var for REACT_APP_MIDGARD_STAGENET_URL)
  • Do we want the pools tab to show some hybrid of stagenet for TC and mainnet for non-TC?

@@ -235,11 +239,13 @@ export const ASGARDEX_SWAP_IDENTIFIER = 999

export const RECOVERY_TOOL_URL: Record<Network, string> = {
testnet: 'https://testnet.thorswap.finance/pending',
stagenet: 'https://stagenet.thorswap.finance/pending',
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@veado Wanted confirmation as to whether this should be stagenet specific or the mainnet value.

Copy link
Collaborator

Choose a reason for hiding this comment

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

It seems https://stagenet.thorswap.finance/pending is accessible, so let's use it.

mainnet: 'https://app.thorswap.finance/pending'
}

export const ASYM_DEPOSIT_TOOL_URL: Record<Network, string> = {
testnet: 'https://testnet.thorswap.finance/',
stagenet: 'https://stagenet.thorswap.finance/',
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@veado See above

Copy link
Collaborator

Choose a reason for hiding this comment

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

Copy link
Collaborator

@pluto9r pluto9r left a comment

Choose a reason for hiding this comment

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

nice, lgtm

.env changes are optional

.env.sample Outdated Show resolved Hide resolved
.env.sample Outdated Show resolved Hide resolved
.env.sample Outdated Show resolved Hide resolved
src/main/api/url.ts Show resolved Hide resolved
@polaris-9r polaris-9r changed the title DRAFT: Issue 1987/add stagenet Issue 1987/add stagenet Dec 31, 2021
@veado
Copy link
Collaborator

veado commented Jan 4, 2022

Wallet -> Assets -> ETH

UNSUPPORTED GETDEFAULTPROVIDER NETWORK (OPERATION="GETDEFAULTPROVIDER", NETWORK="STAGENET", CODE=NETWORK_ERROR, VERSION=PROVIDERS/5.4.5)

Screenshot from 2022-01-04 12-25-58

network should be mainnet(not stagenet) It looks like an issue at latest xchain-ethereum.

@veado
Copy link
Collaborator

veado commented Jan 4, 2022

Wallet -> Assets -> BTC

xhr.js:210 GET https://api.haskoin.com/haskoin-store/btc/address/{address}/balance net::ERR_FAILED 404

Same for chaosnet, It might be an issue with haskoin api. Can we use https://haskoin.ninerealms.com/ ?

Screenshot from 2022-01-04 12-30-14

@veado
Copy link
Collaborator

veado commented Jan 4, 2022

@veado Wanted your perspective on one last part of this PR. So as you can see from the screenshots in my first post, the pool values are different between chaosnet and stagenet because they are hitting the mainnet vs stagenet versions of midgard. I can confirm though that in the wallet tab that the non-TC chain wallet addresses are the same values. Question is:

- Do we want the pools tab to reflect all mainnet pool values? All stagenet pool values? (i.e. simply setting the env. var for `REACT_APP_MIDGARD_STAGENET_URL`)

- Do we want the pools tab to show some hybrid of stagenet for TC and mainnet for non-TC?

I would keep it separate (stagenet -> stagenet pools only, mainnet -> mainnet pools only - but no hybrid) to handle differences more easily. Since we are using different RUNE (thor vs. sthor) addresses, pairing RUNE will be different. Not sure if this might be an issue by mixing stagenet/mainnet pools.

@veado
Copy link
Collaborator

veado commented Jan 4, 2022

It seems viewblock does not support stagenet yet, that's why external links for any tx are going to https://viewblock.io/thorchain instead of https://viewblock.io/thorchain/tx/{hash}?network=stagenet (which is not accessible atm)

Example: pools -> detail

Screenshot from 2022-01-04 18-18-27

Three options:

  • Ignore this issue and just let external tx links go to https://viewblock.io/thorchain
  • Hide all external icons + view transaction links in case of stagenet
  • Fix tx urls to be https://viewblock.io/thorchain/tx/{hash}?network=stagenet (in hope it will be such url)

@polaris-9r
Copy link
Collaborator Author

polaris-9r commented Jan 4, 2022

It seems viewblock does not support stagenet yet, that's why external links for any tx are going to https://viewblock.io/thorchain instead of https://viewblock.io/thorchain/tx/{hash}?network=stagenet (which is not accessible atm)

Example: pools -> detail

Screenshot from 2022-01-04 18-18-27

Three options:

* Ignore this issue and just let external tx links go to `https://viewblock.io/thorchain`

* Hide all external icons + view transaction links in case of `stagenet`

* Fix tx urls to be `https://viewblock.io/thorchain/tx/{hash}?network=stagenet` (in hope it will be such url)

Yeah, given this lack of functionality, I think I'd personally want to err on the side of not showing transactions at all in case of stagenet, in line with what we were saying that stagenet should only see stagenet, mainnet should only see mainnet, etc. I'll put some gatekeeping for now and we can figure out when viewblock is going to support stagenet.

Copy link
Collaborator

@veado veado left a comment

Choose a reason for hiding this comment

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

300fce1 fixes xchain-* related issues. Agree, urls for viewblock are not a blocker - the viewblock team will provide stagenet soon (see Discord message https://discord.com/channels/838986635756044328/846612413477945384/928043598367326228)

Happy to merge now, thanks again for your great work @polaris-9r!

@veado veado merged commit d7bb0a6 into develop Jan 5, 2022
@veado veado deleted the issue-1987/add-stagenet branch January 5, 2022 12:44
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.

3 participants