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.

Sign up for GitHub

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

minor xdefi improvements #423

Merged
merged 5 commits into from
Mar 24, 2022
Merged

minor xdefi improvements #423

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
5ce70aa
xdefi: tree-shakable lodash import
mrnerdhair Feb 12, 2022
e7d256a
xdefi: remove create/info functions
mrnerdhair Feb 12, 2022
0807995
xdefi: typo fixes
mrnerdhair Feb 12, 2022
3054232
integration: improve xdefi mocks
mrnerdhair Feb 12, 2022
cad67f9
xdefi: set provider upon construction
mrnerdhair Mar 23, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
87 changes: 0 additions & 87 deletions integration/src/wallets/mocks/@xdefi/xdefi.ts
View file
Open in desktop

This file was deleted.

97 changes: 92 additions & 5 deletions integration/src/wallets/xdefi.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,19 +1,106 @@
import * as core from "@shapeshiftoss/hdwallet-core";
import * as xdefi from "@shapeshiftoss/hdwallet-xdefi";
import { createMockWallet } from "./mocks/@xdefi/xdefi";

const mockSignEthTxResponse = {
r: "0x122269dc9cffc02962cdaa5af54913ac3e7293c3dd2a8ba7e38da2bc638f92df",
s: "0x36334d475fc12eb62681fb2cb10f177101d5cf4c3a735c94460d92bfa2389cc8",
v: 1,
serialized:
"0x02f872018084540ae4808516854be509825ac394fc0cc6e85dff3d75e3985e0cb83b090cfd498dd1871550f7dca7000080c001a0122269dc9cffc02962cdaa5af54913ac3e7293c3dd2a8ba7e38da2bc638f92dfa036334d475fc12eb62681fb2cb10f177101d5cf4c3a735c94460d92bfa2389cc8",
};

const mockSignEthTxResponse1559 = {
r: "0x63db3dd3bf3e1fe7dde1969c0fc8850e34116d0b501c0483a0e08c0f77b8ce0a",
s: "0x28297d012cccf389f6332415e96ee3fc0bbf8474d05f646e029cd281a031464b",
v: 38,
serialized:
"0xf86b018501dcd650008256229412ec06288edd7ae2cc41a843fe089237fc7354f0872c68af0bb140008026a063db3dd3bf3e1fe7dde1969c0fc8850e34116d0b501c0483a0e08c0f77b8ce0aa028297d012cccf389f6332415e96ee3fc0bbf8474d05f646e029cd281a031464b",
};

const mockSignEthTxResponse1559Optional = {
r: "0x63db3dd3bf3e1fe7dde1969c0fc8850e34116d0b501c0483a0e08c0f77b8ce0a",
s: "0x28297d012cccf389f6332415e96ee3fc0bbf8474d05f646e029cd281a031464b",
v: 38,
serialized:
"0xf86b018501dcd650008256229412ec06288edd7ae2cc41a843fe089237fc7354f0872c68af0bb140008026a063db3dd3bf3e1fe7dde1969c0fc8850e34116d0b501c0483a0e08c0f77b8ce0aa028297d012cccf389f6332415e96ee3fc0bbf8474d05f646e029cd281a031464b",
};

const mockSignERC20Tx = {
r: "0x1238fd332545415f09a01470350a5a20abc784dbf875cf58f7460560e66c597f",
s: "0x10efa4dd6fdb381c317db8f815252c2ac0d2a883bd364901dee3dec5b7d3660a",
v: 37,
serialized:
"0xf8a20114149441e5560054824ea6b0732e656e3ad64e20e94e4580b844a9059cbb0000000000000000000000001d8ce9022f6284c3a5c317f8f34620107214e54500000000000000000000000000000000000000000000000000000002540be40025a01238fd332545415f09a01470350a5a20abc784dbf875cf58f7460560e66c597fa010efa4dd6fdb381c317db8f815252c2ac0d2a883bd364901dee3dec5b7d3660a",
};

const mockSignLongContractData = {
r: "0x5ea245ddd00fdf3958d6223255e37dcb0c61fa62cfa9cfb25e507da16ec8d96a",
s: "0x6c428730776958b80fd2b2201600420bb49059f9b34ee3b960cdcce45d4a1e09",
v: 37,
serialized:
"0xf9063081ab85055ae826008305140e94def1c0ded9bec7f1a1670819833240f027b25eff80b905c8415565b0000000000000000000000000dac17f958d2ee523a2206206994597c13d831ec7000000000000000000000000a0b86991c6218b36c1d19d4a2e9eb0ce3606eb480000000000000000000000000000000000000000000000000000000c5c360b9c0000000000000000000000000000000000000000000000000000000c58cb06ec00000000000000000000000000000000000000000000000000000000000000a00000000000000000000000000000000000000000000000000000000000000002000000000000000000000000000000000000000000000000000000000000004000000000000000000000000000000000000000000000000000000000000003600000000000000000000000000000000000000000000000000000000000000013000000000000000000000000000000000000000000000000000000000000004000000000000000000000000000000000000000000000000000000000000002c000000000000000000000000000000000000000000000000000000000000000200000000000000000000000000000000000000000000000000000000000000000000000000000000000000000dac17f958d2ee523a2206206994597c13d831ec7000000000000000000000000a0b86991c6218b36c1d19d4a2e9eb0ce3606eb4800000000000000000000000000000000000000000000000000000000000001200000000000000000000000000000000000000000000000000000000000000280000000000000000000000000000000000000000000000000000000000000028000000000000000000000000000000000000000000000000000000000000002600000000000000000000000000000000000000000000000000000000c5c360b9c0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000100000000000000000000000000000000000000000000000000000000000000200000000000000000000000000000000a446f646f0000000000000000000000000000000000000000000000000000000000000000000000000000000c5c360b9c0000000000000000000000000000000000000000000000000000000c58cb06ec00000000000000000000000000000000000000000000000000000000000000800000000000000000000000000000000000000000000000000000000000000060000000000000000000000000533da777aedce766ceae696bf90f8541a4ba80eb000000000000000000000000c9f93163c99695c6526b799ebca2207fdf7d61ad0000000000000000000000000000000000000000000000000000000000000001000000000000000000000000000000000000000000000000000000000000000100000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000007000000000000000000000000000000000000000000000000000000000000004000000000000000000000000000000000000000000000000000000000000001000000000000000000000000000000000000000000000000000000000000000020000000000000000000000000000000000000000000000000000000000000004000000000000000000000000000000000000000000000000000000000000000c00000000000000000000000000000000000000000000000000000000000000003000000000000000000000000dac17f958d2ee523a2206206994597c13d831ec7000000000000000000000000a0b86991c6218b36c1d19d4a2e9eb0ce3606eb48000000000000000000000000eeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeeee0000000000000000000000000000000000000000000000000000000000000000869584cd000000000000000000000000c770eefad204b5180df6a14ee197d99d808ee52d0000000000000000000000000000000000000000000000da413736cc60c8dd4e25a05ea245ddd00fdf3958d6223255e37dcb0c61fa62cfa9cfb25e507da16ec8d96aa06c428730776958b80fd2b2201600420bb49059f9b34ee3b960cdcce45d4a1e09",
};

export function name(): string {
return "XDeFi";
}

export async function createWallet(): Promise<core.HDWallet> {
const wallet = createMockWallet();
if (!wallet) throw new Error("No XDeFi wallet found");
// mock xdefi
(globalThis as any).xfi = {
Copy link
Contributor

Choose a reason for hiding this comment

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

packages/hdwallet-xdefi/src/adapter.ts has the only occurrence of globalthis as any in the codebase currently - don't we have any way to avoid these two anys?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We can write type definitions for xdefi; explicit anys are prevalent in this repo, though, and I think it would make more sense to address them as a group. My plan is to get eslint in with the no-explicit-any rule turned off (#453) and then handle turning it on as a separate issue/pr.

ethereum: {
request: jest.fn(({ method, params }: any) => {
switch (method) {
case "eth_accounts":
case "eth_requestAccounts":
return ["0x3f2329C9ADFbcCd9A84f52c906E936A42dA18CB8"];
case "personal_sign":
const [message] = params;

if (message === "48656c6c6f20576f726c64")
return "0x29f7212ecc1c76cea81174af267b67506f754ea8c73f144afa900a0d85b24b21319621aeb062903e856352f38305710190869c3ce5a1425d65ef4fa558d0fc251b";

throw new Error("unknown message");
case "eth_sendTransaction":
const [{ to }] = params;

return `txHash-${to}`;
default:
throw new Error(`ethereum: Unknown method ${method}`);
}
})
}
};

const adapter = xdefi.XDeFiAdapter.useKeyring(new core.Keyring())
const wallet = await adapter.pairDevice()

wallet.ethSignTx = jest
.fn()
.mockReturnValueOnce(mockSignEthTxResponse)
.mockReturnValueOnce(mockSignEthTxResponse1559)
.mockReturnValueOnce(mockSignEthTxResponse1559Optional)
.mockReturnValueOnce(mockSignERC20Tx)
.mockReturnValueOnce(mockSignLongContractData);

wallet.ethSignMessage = jest.fn().mockReturnValue({
address: "0x3f2329C9ADFbcCd9A84f52c906E936A42dA18CB8",
signature:
"0x29f7212ecc1c76cea81174af267b67506f754ea8c73f144afa900a0d85b24b21319621aeb062903e856352f38305710190869c3ce5a1425d65ef4fa558d0fc251b",
});

wallet.ethVerifyMessage = jest.fn().mockReturnValue({
address: "0x3f2329C9ADFbcCd9A84f52c906E936A42dA18CB8",
message: "Hello World",
signature:
"0x29f7212ecc1c76cea81174af267b67506f754ea8c73f144afa900a0d85b24b21319621aeb062903e856352f38305710190869c3ce5a1425d65ef4fa558d0fc251b",
});

return wallet;
}

export function createInfo(): core.HDWalletInfo {
return xdefi.info();
return new xdefi.XDeFiHDWalletInfo();
}

export function selfTest(get: () => core.HDWallet): void {
Expand All @@ -30,7 +117,7 @@ export function selfTest(get: () => core.HDWallet): void {
expect(await wallet.ethSupportsNetwork(1)).toEqual(true);
});

it("prepends xdefi: to the eth address to create the deviceId", async () => {
it("prepends xDeFi: to the eth address to create the deviceId", async () => {
if (!wallet) return;
expect(await wallet.getDeviceID()).toEqual("xDeFi:0x3f2329C9ADFbcCd9A84f52c906E936A42dA18CB8");
});
Expand Down
4 changes: 2 additions & 2 deletions packages/hdwallet-xdefi/src/adapter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@ export class XDeFiAdapter {
console.error("Could not get XDeFi accounts. ");
throw error;
}
const wallet = new XDeFiHDWallet();
await wallet.initialize(provider);
const wallet = new XDeFiHDWallet(provider);
await wallet.initialize();
const deviceID = await wallet.getDeviceID();
this.keyring.add(wallet, deviceID);
this.currentDeviceID = deviceID;
Expand Down
18 changes: 5 additions & 13 deletions packages/hdwallet-xdefi/src/xdefi.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
import * as core from "@shapeshiftoss/hdwallet-core";
import { create, XDeFiHDWallet } from ".";

import * as xdefi from "./xdefi";
import { XDeFiHDWallet, XDeFiHDWalletInfo } from ".";

describe("XDeFIHDWalletInfo", () => {
const info = new XDeFiHDWalletInfo();

it("should have correct metadata", async () => {
const info = xdefi.info();
expect(info.getVendor()).toBe("XDeFi");
expect(info.hasOnDevicePinEntry()).toBe(false);
expect(info.hasOnDevicePassphrase()).toBe(false);
Expand All @@ -19,7 +18,6 @@ describe("XDeFIHDWalletInfo", () => {
expect(await info.supportsBroadcast()).toBe(true);
});
it("should produce correct path descriptions", () => {
const info = xdefi.info();
expect(info.hasNativeShapeShift()).toBe(false);
[
{
Expand All @@ -31,10 +29,10 @@ describe("XDeFIHDWalletInfo", () => {
});
});

describe("XDeFiWHDWallet", () => {
describe("XDeFiHDWallet", () => {
let wallet: XDeFiHDWallet;
beforeEach(() => {
wallet = new XDeFiHDWallet();
wallet = new XDeFiHDWallet(core.untouchable("XDeFiHDWallet:provider"));
wallet.ethAddress = "0x73d0385F4d8E00C5e6504C6030F47BF6212736A8";
});

Expand All @@ -53,7 +51,6 @@ describe("XDeFiWHDWallet", () => {
});

it("should test ethSignTx", async () => {
const wallet = new XDeFiHDWallet();
wallet.ethAddress = "0x123";
wallet.provider = {
request: jest.fn().mockReturnValue({
Expand Down Expand Up @@ -208,9 +205,4 @@ describe("XDeFiWHDWallet", () => {
})
).toEqual(true);
});

it("should create instance of XDeFiHD wallet", () => {
const wallet = create();
expect(wallet).toBeInstanceOf(XDeFiHDWallet);
});
});
22 changes: 6 additions & 16 deletions packages/hdwallet-xdefi/src/xdefi.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import * as core from "@shapeshiftoss/hdwallet-core";
import * as eth from "./ethereum";
import _ from "lodash";
import isObject from "lodash/isObject";

class XDeFiTransport extends core.Transport {
public async getDeviceID() {
Expand All @@ -11,7 +11,7 @@ class XDeFiTransport extends core.Transport {
}

export function isXDeFi(wallet: core.HDWallet): wallet is XDeFiHDWallet {
return _.isObject(wallet) && (wallet as any)._isXDeFi;
return isObject(wallet) && (wallet as any)._isXDeFi;
Copy link
Contributor

Choose a reason for hiding this comment

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

Nitpick: If we're going to import some lodash util, might as well use _.has() which will handle the non-object cases

Copy link
Contributor Author

@mrnerdhair mrnerdhair Mar 23, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Good point -- though IMO consistency with the rest of the wallets' implementations of this type of guard is valuable, and the endgame for them should just be to nuke them wholesale (see #466 / #435).

}

export class XDeFiHDWallet implements core.HDWallet, core.ETHWallet {
Expand All @@ -24,8 +24,9 @@ export class XDeFiHDWallet implements core.HDWallet, core.ETHWallet {
ethAddress?: string | null;
provider: any;

constructor() {
constructor(provider: unknown) {
this.info = new XDeFiHDWalletInfo();
this.provider = provider
}

async getFeatures(): Promise<Record<string, any>> {
Expand All @@ -48,11 +49,8 @@ export class XDeFiHDWallet implements core.HDWallet, core.ETHWallet {
return "XDeFi";
}

public initialize(): never;
public initialize(provider: unknown): Promise<any>;
public async initialize(provider?: unknown): Promise<any> {
if (!provider) throw new Error("provider is required");
this.provider = provider;
public async initialize(): Promise<void> {
// nothing to initialize
}

public hasOnDevicePinEntry(): boolean {
Expand Down Expand Up @@ -274,11 +272,3 @@ export class XDeFiHDWalletInfo implements core.HDWalletInfo, core.ETHWalletInfo
return eth.ethGetAccountPaths(msg);
}
}

export function info() {
return new XDeFiHDWalletInfo();
}

export function create(): XDeFiHDWallet {
return new XDeFiHDWallet();
}