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

ETH Net props for Callisto #35

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

yograterol
Copy link

We're working on integrating Callisto Network in Nifty Wallet and we must start from this project first adding relevant info about RPC, explorer and Chain Code.

@coveralls
Copy link

coveralls commented Mar 29, 2020

Pull Request Test Coverage Report for Build 90

  • 8 of 9 (88.89%) changed or added relevant lines in 3 files are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage decreased (-1.5%) to 95.69%

Changes Missing Coverage Covered Lines Changed/Added Lines %
helpers/get-explorer-links.js 5 6 83.33%
Files with Coverage Reduction New Missed Lines %
helpers/get-explorer-links.js 1 95.0%
Totals Coverage Status
Change from base Build 87: -1.5%
Covered Lines: 129
Relevant Lines: 134

💛 - Coveralls

Copy link
Collaborator

@vbaranov vbaranov left a comment

Choose a reason for hiding this comment

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

@yograterol I left the comments. Some links should be fixed. The rest looks good to me.

it(`${claimPrefix} Ethereum Classic`, () => {
assert.equal(explorerLinks.getExplorerAccountLinkFor('0xd8fe15886d2dcbc5d7c06394beb417aadaf1eee0', '61'), 'https://blockscout.com/etc/mainnet/address/0xd8fe15886d2dcbc5d7c06394beb417aadaf1eee0')
it(`${claimPrefix} Callisto Network`, () => {
assert.equal(explorerLinks.getExplorerAccountLinkFor('0xd8fe15886d2dcbc5d7c06394beb417aadaf1eee0', 820), 'https://explorer2.callisto.network/address/0xd8fe15886d2dcbc5d7c06394beb417aadaf1eee0')
Copy link
Collaborator

Choose a reason for hiding this comment

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

I've checked that address page in Callisto explorer is available at /addr/... path and not at /address/. I suppose path should be changed here in the tests and in the logic.

@@ -83,6 +83,9 @@ describe('eth-net-props', () => {
it(`${claimPrefix} Ethereum Classic`, () => {
assert.equal(explorerLinks.getExplorerTxLinkFor('0x430c90335b32fdcd92e54991668023d58b72bce836e204a81c6d97506c7137e5', 61), 'https://blockscout.com/etc/mainnet/tx/0x430c90335b32fdcd92e54991668023d58b72bce836e204a81c6d97506c7137e5')
})
it(`${claimPrefix} Callisto Network`, () => {
assert.equal(explorerLinks.getExplorerTxLinkFor('0x430c90335b32fdcd92e54991668023d58b72bce836e204a81c6d97506c7137e5', 820), 'https://explorer2.callisto.network/tx/0x430c90335b32fdcd92e54991668023d58b72bce836e204a81c6d97506c7137e5')
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would change to the really existent tx hash in Callisto chain. The current one leads to 404 page.

@@ -119,6 +122,9 @@ describe('eth-net-props', () => {
it(`${claimPrefix} Ethereum Classic`, () => {
assert.equal(explorerLinks.getExplorerTokenLinkFor('0x1ac1c8b874c7b889113a036ba443b082554be5d0', '0xdb23145b64D0E1e15dedf47abd77cCaf3F2327d7', 61), 'https://blockscout.com/etc/mainnet/address/0xdb23145b64D0E1e15dedf47abd77cCaf3F2327d7/tokens/0x1ac1c8b874c7b889113a036ba443b082554be5d0/token_transfers')
})
it(`${claimPrefix} Callisto Network`, () => {
assert.equal(explorerLinks.getExplorerTokenLinkFor('0x1ac1c8b874c7b889113a036ba443b082554be5d0', '0xdb23145b64D0E1e15dedf47abd77cCaf3F2327d7', 820), 'https://explorer2.callisto.network/address/0x1ac1c8b874c7b889113a036ba443b082554be5d0')
Copy link
Collaborator

Choose a reason for hiding this comment

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

The same comment regarding incorrect path to address of the token

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.

3 participants