woodser / monero-ts

TypeScript library for using Monero
http://woodser.github.io/monero-ts/typedocs
MIT License
198 stars 67 forks source link

html5-fs is not compatible with monero-ts #201

Open mainnet-pat opened 2 months ago

mainnet-pat commented 2 months ago

I've been trying to make MoneroWalletFull work in browser and have found out that html5-fs is not compatible with node:fs and it never worked for me in browser. I have experimented with indexeddb-fs as an alternative. Made some tweaks to it and now it is a drop in replacement for node:fs in my next.js app.

See my webpack config for a monorepo:

import { dirname, join } from 'path';
import { fileURLToPath } from 'url';

const __dirname = dirname(fileURLToPath(import.meta.url));

/** @type {import('next').NextConfig} */
const nextConfig = {
  webpack: (config, { isServer }) => {
    config.experiments = { ...config.experiments, topLevelAwait: true }

    if (!isServer) {
      config.resolve.alias.fs = join(__dirname, "../../node_modules/indexeddb-fs")
      config.resolve.fallback.child_process = false;
    }

    config.externals = [...config.externals, ({ request }, callback) => {
      if (/^web-worker$/.test(request)) {
        return callback(null, request);
      }
      callback();
    }];

    return config;
  },
};

export default nextConfig;
woodser commented 2 months ago

Does this depend on https://github.com/woodser/monero-ts/pull/200?

Should the webpack config in monero-ts be updated?

I have a sample next.js app which I'd like to keep updated for others to reference. Should that be updated as well?

I assume this won't help with the "Failed to parse URL from monero-ts/dist/dist/monero_wallet_keys.wasm" issue?

Thanks for your help.

mainnet-pat commented 2 months ago

Hey, #200 is complimentary to this issue, it updates monero-ts to use async-only fs calls. also the drop in fs for browser must be interface-level compatible with node's fs.

Since html5-fs is not compatible with node's fs, I think monero-ts can remove it from its dependencies. Updating monero-ts to use indexeddb-fs would be good, but it is currently not available on npm, it is installed by yarn add https://github.com/mainnet-pat/indexeddb-fs#2.1.5. Do you want it to be available on npm? I am not sure at which namespace to publish it though. @mainnet-pat/indexeddb-fs or rename the project and publish under new name?

I assume this won't help with the "Failed to parse URL from monero-ts/dist/dist/monero_wallet_keys.wasm" issue?

No it is unrelated to this issue

mainnet-pat commented 2 months ago

I assume this won't help with the "Failed to parse URL from monero-ts/dist/dist/monero_wallet_keys.wasm" issue?

This issue could be monkey patched to disable fetch in webworker glue code. What I did not try but suppose it could work is that monero-ts/dist/dist/monero_wallet_keys.wasm should look like file://monero-ts/dist/dist/monero_wallet_keys.wasm

woodser commented 2 months ago

but it is currently not available on npm

The readme indicates it can be installed from npm: https://github.com/playerony/indexeddb-fs?tab=readme-ov-file#installation

This doesn't work?

mainnet-pat commented 2 months ago

It does not, I have patched it to behave in a compatible way with node's fs - to replace the target file on renames and moves instead of failing. monero-ts assumes the target file is overwritten upon move.

what you can do to use the npm's indexed-db is not to rely on this move with overwrite behaviour and first delete the target file then move the wallet.new to wallet file

mainnet-pat commented 2 months ago

also I think it is viable to protect this file handling calls with a mutex from async-mutex package, so that concurrent save calls would not yield errors. I have experienced them

woodser commented 2 months ago

what you can do to use the npm's indexed-db is not to rely on this move with overwrite behaviour and first delete the target file then move the wallet.new to wallet file

I think we should take this route, but somehow make a backup of the original file before it's replaced by the new, just in case writing the new file fails.

woodser commented 2 months ago

also I think it is viable to protect this file handling calls with a mutex from async-mutex package, so that concurrent save calls would not yield errors. I have experienced them

Would make sense. I think we should open a new issue for this.

Created an issue for this: https://github.com/woodser/monero-ts/issues/204

mainnet-pat commented 2 months ago

I think we should take this route, but somehow make a backup of the original file before it's replaced by the new, just in case writing the new file fails.

cool, yeah

also unlink should be invoked on target files before rename

woodser commented 2 months ago

Maybe https://github.com/woodser/monero-ts/pull/200 should include these changes and the dependency switch from html5-fs to indexeddb-fs?

mainnet-pat commented 2 months ago

I can do that, yeah

mainnet-pat commented 2 months ago

I've created a PR for the next example and updated the #200. I could not run the entire test suite locally since it requires some setup, having a github CI action with dockerized monero demon and wallet cli would be nice to have.

woodser commented 2 months ago

Thanks, yeah simplifying the setup for running tests is definitely needed.

I was thinking at least using Make to automate the setup, but maybe docker would be good too.

A separate issue should be opened for this and bounty added.