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

Adds Create Account flow; Snapshot Download updates #28

Merged
merged 7 commits into from
Nov 9, 2023
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
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
30 changes: 30 additions & 0 deletions main/api/accounts/handleCreateAccount.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
import { AccountFormat } from "@ironfish/sdk";

import { manager } from "../manager";

export async function handleCreateAccount({ name }: { name: string }) {
const ironfish = await manager.getIronfish();
const rpcClient = await ironfish.rpcClient();

const createResponse = await rpcClient.wallet.createAccount({
name,
});

const exportResponse = await rpcClient.wallet.exportAccount({
account: createResponse.content.name,
viewOnly: false,
format: AccountFormat.Mnemonic,
});

const mnemonic = exportResponse.content.account?.toString();

if (!mnemonic) {
throw new Error("Failed to get mnemonic phrase");
}

return {
name: createResponse.content.name,
publicAddress: createResponse.content.publicAddress,
mnemonic,
};
}
29 changes: 29 additions & 0 deletions main/api/accounts/handleExportAccount.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
import { AccountFormat } from "@ironfish/sdk";
import * as z from "zod";

import { manager } from "../manager";

export const handleExportAccountInputs = z.object({
name: z.string(),
format: z.custom<`${AccountFormat}`>((format) => {
return typeof format === "string" && format in AccountFormat;
}),
viewOnly: z.boolean().optional(),
});

export async function handleExportAccount({
name,
format,
viewOnly = false,
}: z.infer<typeof handleExportAccountInputs>) {
const ironfish = await manager.getIronfish();
const rpcClient = await ironfish.rpcClient();

const exportResponse = await rpcClient.wallet.exportAccount({
account: name,
format: AccountFormat[format],
viewOnly,
});

return exportResponse.content;
}
23 changes: 23 additions & 0 deletions main/api/accounts/handleRenameAccount.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import * as z from "zod";

import { manager } from "../manager";

export const handleRenameAccountInputs = z.object({
account: z.string(),
newName: z.string(),
});

export async function handleRenameAccount({
account,
newName,
}: z.infer<typeof handleRenameAccountInputs>) {
const ironfish = await manager.getIronfish();
const rpcClient = await ironfish.rpcClient();

const renameResponse = await rpcClient.wallet.renameAccount({
account,
newName,
});

return renameResponse.content;
}
28 changes: 28 additions & 0 deletions main/api/accounts/index.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,16 @@
import { z } from "zod";

import { handleCreateAccount } from "./handleCreateAccount";
import {
handleExportAccount,
handleExportAccountInputs,
} from "./handleExportAccount";
import { handleGetAccount } from "./handleGetAccount";
import { handleGetAccounts } from "./handleGetAccounts";
import {
handleRenameAccountInputs,
handleRenameAccount,
} from "./handleRenameAccount";
import { manager } from "../manager";
import { t } from "../trpc";

Expand All @@ -16,6 +25,25 @@ export const accountRouter = t.router({
return handleGetAccount(opts.input);
}),
getAccounts: t.procedure.query(handleGetAccounts),
createAccount: t.procedure
.input(
z.object({
name: z.string(),
}),
)
.mutation(async (opts) => {
return handleCreateAccount(opts.input);
}),
exportAccount: t.procedure
.input(handleExportAccountInputs)
.query(async (opts) => {
return handleExportAccount(opts.input);
}),
renameAccount: t.procedure
.input(handleRenameAccountInputs)
.mutation(async (opts) => {
return handleRenameAccount(opts.input);
}),
isValidPublicAddress: t.procedure
.input(
z.object({
Expand Down
45 changes: 25 additions & 20 deletions main/api/ironfish/Ironfish.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { BoxKeyPair } from "@ironfish/rust-nodejs";
import {
ALL_API_NAMESPACES,
FullNode,
IronfishSdk,
NodeUtils,
RpcClient,
Expand All @@ -9,10 +10,10 @@ import {
} from "@ironfish/sdk";
import { v4 as uuid } from "uuid";

import { logger } from "./logger";
import packageJson from "../../../package.json";
import { SnapshotManager } from "../snapshot/snapshotManager";
import { SplitPromise, splitPromise } from "../utils";
import { logger } from "./logger";

function getPrivateIdentity(sdk: IronfishSdk) {
const networkIdentity = sdk.internal.get("networkIdentity");
Expand All @@ -30,6 +31,7 @@ export class Ironfish {

private rpcClientPromise: SplitPromise<RpcClient> = splitPromise();
private sdkPromise: SplitPromise<IronfishSdk> = splitPromise();
private fullNodePromise: SplitPromise<FullNode> = splitPromise();
private _started: boolean = false;
private _initialized: boolean = false;
private _dataDir: string;
Expand All @@ -46,19 +48,25 @@ export class Ironfish {
return this.sdkPromise.promise;
}

fullNode(): Promise<FullNode> {
return this.fullNodePromise.promise;
}

async downloadSnapshot(): Promise<void> {
if (this._started) {
throw new Error("Cannot download snapshot after node has started");
}

const sdk = await this.sdk();
await this.snapshotManager.run(sdk);
const node = await this.fullNode();
await this.snapshotManager.run(sdk, node);
}

async init() {
if (this._initialized) {
return;
}

this._initialized = true;

console.log("Initializing IronFish SDK...");
Expand All @@ -69,22 +77,6 @@ export class Ironfish {
pkg: getPackageFrom(packageJson),
});

this.sdkPromise.resolve(sdk);
}

async start() {
if (this._started) {
return;
}
this._started = true;

if (this.snapshotManager.started) {
await this.snapshotManager.result();
}

console.log("Starting IronFish Node...");

const sdk = await this.sdk();
const node = await sdk.node({
privateIdentity: getPrivateIdentity(sdk),
autoSeed: true,
Expand All @@ -104,13 +96,26 @@ export class Ironfish {
await node.internal.save();
}

await node.start();

const rpcClient = new RpcMemoryClient(
node.logger,
node.rpc.getRouter(ALL_API_NAMESPACES),
);

this.sdkPromise.resolve(sdk);
this.fullNodePromise.resolve(node);
this.rpcClientPromise.resolve(rpcClient);
}

async start() {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I updated the start command so that init creates the Ironfish FullNode, so we can query for accounts on startup.

Now, start just starts the node.

if (this._started) {
return;
}

console.log("Starting FullNode...");
dgca marked this conversation as resolved.
Show resolved Hide resolved

this._started = true;

const node = await this.fullNode();
await node.start();
}
}
2 changes: 1 addition & 1 deletion main/api/ironfish/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,6 @@ export const ironfishRouter = t.router({
}),
startNode: t.procedure.mutation(async () => {
const ironfish = await manager.getIronfish();
ironfish.start();
await ironfish.start();
}),
});
23 changes: 16 additions & 7 deletions main/api/manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import {
} from "./user-settings/userSettings";

export type InitialState =
| "create-account"
| "onboarding"
| "snapshot-download-prompt"
| "start-node";

Expand Down Expand Up @@ -36,15 +36,24 @@ export class Manager {
if (this._initialState) return this._initialState;

const ironfish = await this.getIronfish();
const sdk = await ironfish.sdk();
const rpcClient = await ironfish.rpcClient();

if (sdk.internal.get("isFirstRun")) {
this._initialState = "snapshot-download-prompt";
} else {
this._initialState = "start-node";
const accountsResponse = await rpcClient.wallet.getAccounts();

if (accountsResponse.content.accounts.length === 0) {
return "onboarding";
}

const statusResponse = await rpcClient.node.getStatus();
const headTimestamp = statusResponse.content.blockchain.headTimestamp;
const hoursSinceLastBlock = (Date.now() - headTimestamp) / 1000 / 60 / 60;

// If the last block was more than 2 weeks ago, prompt the user to download a snapshot
if (hoursSinceLastBlock > 24 * 7 * 2) {
return "snapshot-download-prompt";
}

return this._initialState;
return "start-node";
}
}

Expand Down
11 changes: 5 additions & 6 deletions main/api/snapshot/snapshotManager.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import fsAsync from "fs/promises";

import { Event, IronfishSdk, Meter, NodeUtils } from "@ironfish/sdk";
import { Event, FullNode, IronfishSdk, Meter } from "@ironfish/sdk";
import log from "electron-log";

import {
Expand All @@ -16,15 +16,15 @@ export class SnapshotManager {
snapshotPromise: SplitPromise<void> = splitPromise();
started = false;

async run(sdk: IronfishSdk): Promise<void> {
async run(sdk: IronfishSdk, node: FullNode): Promise<void> {
if (this.started) {
return;
}

this.started = true;

try {
await this._run(sdk);
await this._run(sdk, node);
this.snapshotPromise.resolve();
} catch (e) {
this.snapshotPromise.reject(e);
Expand All @@ -35,9 +35,7 @@ export class SnapshotManager {
return this.snapshotPromise.promise;
}

async _run(sdk: IronfishSdk): Promise<void> {
const node = await sdk.node();
await NodeUtils.waitForOpen(node);
async _run(sdk: IronfishSdk, node: FullNode): Promise<void> {
const nodeChainDBVersion = await node.chain.blockchainDb.getVersion();
await node.closeDB();

Expand Down Expand Up @@ -112,5 +110,6 @@ export class SnapshotManager {

await downloadedSnapshot.replaceDatabase();
await fsAsync.rm(downloadedSnapshot.file);
await node.openDB();
Copy link
Member

Choose a reason for hiding this comment

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

Thinking about the pattern we want here for the node. There's 2 steps that require the node database to be open before we start the node: snapshot and getAccounts.

In this PR we open the node initially in Ironfish.init and then pass it to the snapshot to close it and then make sure it's open when it finishes. I could see it this way. I could also see not having the node open initially and then each individual process snapshot and getAccounts create their own node instance, open it and make sure it's close when its finished.

Personally a process opening its own node and making sure to close down at the end makes a little more sense to me than getting an open node and then making sure it remains open at the end. Wdyt?

Copy link
Member

Choose a reason for hiding this comment

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

If each step opened their own node we could remove the fullNode promise. But then we'd also have to create a 2 memory clients. One ephemeral one to get the accounts and then another one on ironfish.start() So guess there are pros/cons to each approach

Copy link
Member

Choose a reason for hiding this comment

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

I think I might want to refactor snapshotManager to take an Ironfish (the node app class) instance rather than a separate SDK and node. I think my thoughts here are similar to yours, but with different reasoning. It generally seems like it leads to confusing code if we have operations on the node itself (not through the RPC client) spread throughout the codebase, because you have to handle a bunch of node states, like whether the DB is open and whether the node is stopped/started.

Since the DB can only be held by one node at a time, I think the node instance naturally tends towards being a singleton, vs having per-process nodes in e.g. snapshot and getAccounts.

As an aside, we end up needing the fullNode promise for shutting down the node (and I think it’s generally worthwhile to hold an explicit handle to the node, since it’s running in the background anyway)

TL;DR, I think you're right that each step should make sure the node is in the necessary state, rather than the prior step needing to leave the node in a particular state. But I don't really want to slow this PR down unless it's causing bugs -- I figure I can go refactor it pretty easily if we run into issues at that point.

Copy link
Member

Choose a reason for hiding this comment

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

Yea agreed. Talked about this and I think keeping the fullNode as a singleton on Ironfish makes sense

}
}
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,9 @@
"react": "^18.2.0",
"react-dom": "^18.2.0",
"react-hook-form": "^7.47.0",
"react-icons": "^4.11.0",
"type-fest": "^4.6.0",
"typescript": "^4.9.5",
"typescript": "^5.2.2",
"usehooks-ts": "^2.9.1",
"zod": "^3.22.3"
},
Expand Down
Loading