-
Notifications
You must be signed in to change notification settings - Fork 91
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
Feature: Add MayaChain (CACAO) #662
base: master
Are you sure you want to change the base?
Conversation
…feature-maya-final � Conflicts: � packages/hdwallet-native/package.json
Feature maya final
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
return; | ||
} | ||
// eslint-disable-next-line no-console | ||
console.log(core); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
?
} | ||
// eslint-disable-next-line no-console | ||
console.log(core); | ||
if (true) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
?
$mayachainNativeResults.val(result); | ||
} else { | ||
const label = await wallet.getLabel(); | ||
$mayachainNativeResults.val(label + " does not support THORChain"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
?
showDisplay: true, | ||
}); | ||
$mayachainNativeResults.val(result); | ||
} else { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pretty sure this is never going to it as the else block of if (true)
return; | ||
} | ||
if (true) { | ||
// eslint-disable-next-line no-console |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we sure about disabling this rule here?
$mayachainNativeResults.val("No wallet?"); | ||
return; | ||
} | ||
if (true) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also what?
@@ -136,6 +137,14 @@ export function integration(suite: WalletSuite): void { | |||
thorchainTests(() => ({ wallet, info })); | |||
}); | |||
|
|||
describe("mayachainTests", () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't follow the naming convention of tests in this file. Shouldn't this be MayaChainWallet
?
@@ -136,6 +137,14 @@ export function integration(suite: WalletSuite): void { | |||
thorchainTests(() => ({ wallet, info })); | |||
}); | |||
|
|||
describe("mayachainTests", () => { | |||
beforeAll(async () => { | |||
wallet = await suite.createWallet("Thorchain"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this correct?
@@ -0,0 +1,7 @@ | |||
import * as core from "@shapeshiftoss/hdwallet-core"; | |||
|
|||
import { mayachainTests as tests } from "./mayachain"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you follow the naming convention of the integrations module, you won't have to alias the import
if (!wallet) return; | ||
|
||
const out = wallet.describePath({ | ||
path: core.bip32ToAddressNList("m/44'/931'/0'/0/0"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ditto path question here
@@ -40,6 +40,7 @@ | |||
"@types/inquirer": "9.0.3", | |||
"@typescript-eslint/eslint-plugin": "^6.7.3", | |||
"@typescript-eslint/parser": "^6.7.3", | |||
"browserify-zlib": "^0.2.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need this as a direct dep?
@@ -158,6 +158,8 @@ const slip44Table = Object.freeze({ | |||
Gnosis: 60, | |||
Arbitrum: 60, | |||
ArbitrumNova: 60, | |||
Mayachain: 931, | |||
Cacao: 931, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need this? Isn't CACAO the native asset of MAYA Chain?
@@ -1,6 +1,6 @@ | |||
{ | |||
"name": "@shapeshiftoss/hdwallet-native", | |||
"version": "1.53.3", | |||
"version": "1.52.14", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
?
@@ -11,7 +11,7 @@ import * as util from "./util"; | |||
|
|||
const ATOM_CHAIN = "cosmoshub-4"; | |||
|
|||
const protoTxBuilder = PLazy.from(() => import("@shapeshiftoss/proto-tx-builder")); | |||
const protoTxBuilder = PLazy.from(() => import("@keepkey/proto-tx-builder")); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Revert. Except for hdwallet-keepkey, we don't import @keepkey namespace packages in hdwallet.
"memo": "", | ||
"msg": [ | ||
{ | ||
"type": "mayachain/MsgDeposit", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
using amino type for proto tx
"memo": "", | ||
"msg": [ | ||
{ | ||
"type": "mayachain/MsgSend", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ditto
fix(common): remove unused packages
Revert "fix(common): remove unused packages"
Overview
This pull request introduces support for MayaChain in the HDWallet package. Additionally, it re-enables Osmosis support, which was previously removed.
Changes
MayaChain Integration
Re-enable Osmosis
Rationale
Testing
Notes
Conclusion
This PR is a significant step towards making the HDWallet more versatile and robust. I look forward to the community's feedback/
Request for comments and review: @BitHighlander. 🤖 generated pr