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

Fix AVAX, include AVAXC #71

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

Conversation

0xcharchar
Copy link

@0xcharchar 0xcharchar commented Apr 24, 2022

The current AVAX implementation is actually for the C-Chain, which is actually AVAXC according to SLIP-0044.

These changes are breaking but accomplish two things:

  • Fix the AVAX implementation to parse X- & P- chains instead of the C-Chain
  • Add in AVAXC support to continue parsing the C-Chain

A couple questions I have:

  • Would you like the dist/ folder committed as well?
  • Would you prefer the ID stripping to be its own function? My thinking was that it was unnecessary abstraction to make it a function

References:

According to BIP-0044 and SLIP-0044, AVAX is a reference to the
Avalanche X- & P- chains. This adds a validator to strip away the
optional ID (X- or P-) and then validates using the BIP173 validator.

This is a breaking change but the correct behaviour.
The Avalanche C-Chain is Ethereum compatible but it is part of the spec
that it can be prepended with an ID of C-. This validator strips that ID
before passing to the Ethereum validator.
@0xcharchar
Copy link
Author

The error in Travis looks like a problem with Node 18 and Travis, not the changes here.

@0xcharchar
Copy link
Author

Sorry to ping you @christsim, is there any chance I can get a review on this?

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.

1 participant