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

createWalletFull, importOutputs and exportKeyImages cannot function correctly when a deposit is made to a subaddress with large index #99

Closed
will-yjn opened this issue Aug 2, 2022 · 10 comments

Comments

@will-yjn
Copy link

will-yjn commented Aug 2, 2022

Hi there,

I use the view-only wallet plus offline wallet mechanism to sign keyImages.

image

I use try catchs to locate the error to be throw by the last line above: TypeError: (intermediate value) is not iterable. I check the monero-javascript repo and doubt if it is thrown by the line in the red box below:

Screen Shot 2022-08-02 at 15 38 11

..., because a null value is not iterable.

From my understanding that._module.export_key_images(that._cppAddress, all, callback); calls the wasm function which was translated from c++. Is there a way to debug the wasm code? Below is an excerpt from monero_wasm_bridge.cpp:

Screen Shot 2022-08-02 at 15 42 04

I suppose something went wrong at this line doc.AddMember("keyImages", monero_utils::to_rapidjson_val(doc.GetAllocator(), key_images), doc.GetAllocator()); Somehow the doc object cannot locate memory or something, so the later half returns null. Do you know what causes the original error when exportKeyImages, or what could cause doc.AddMember to return a bug?

@will-yjn
Copy link
Author

will-yjn commented Aug 4, 2022

Updates:
Screen Shot 2022-08-04 at 14 42 26

I print the number of outputs and the decoded outputs, the result is:
numOfOutputs: undefined; outputs: {"amount":"0","index":0,"accountIndex":0,"subaddressIndex":0,"isSpent":false,"isFrozen":false}

It seems the outputs have not been imported successfully. Or the outputs hex itself is incorrect. But the view-only wallet correctly shows the balances, transfers, etc. What could possibly cause view-only wallet to export incorrect output? Or, since outputhex is exported from one machine, and importing outputhex and export key images is from another machine, could the two machines' environment cause this?

@woodser
Copy link
Owner

woodser commented Aug 7, 2022

I still wasn't able to recreate the error. As far as I can tell, the library code should always initialize this field from export_key_images().

This suggests an unexpected error is occurring in wallet2's export_key_images() which isn't being caught. I've added the export/import functions to the exception whitelist (wasm_exception_whitelist.txt) so the library catches any errors from those functions in the new v0.7.2 release.

If you're running against v0.18.0.0, there's at least one known issue with the output export/import but it may be unrelated.

@will-yjn
Copy link
Author

will-yjn commented Aug 11, 2022

@woodser I found the bug. I made two deposits: one at the primary address, one at the subaddress with index 181162. If I set up the view-only wallet to scan just 1000 subaddresses, thus skipping the one at index 181162, the outputhex contains only 1 utxo and can be imported into the offline wallet, and key images can be exported without error. But if I set up the view-only wallet to scan 1 million subaddresses, thus including the one at index 181162, the outputhex will be much longer and cannot be properly imported into the offline wallet, and key images cannot be exported (see above error).

So my question is, when instantiating the offline wallet with createWalletFull, can I specify the subaddress to be included? If there is none, then it seems to be a bug.

@will-yjn will-yjn changed the title exportKeyImages Error: TypeError: (intermediate value) is not iterable createWalletFull, importOutputs and exportKeyImages cannot function correctly when a deposit is made to a subaddress with large index Aug 11, 2022
@woodser
Copy link
Owner

woodser commented Aug 18, 2022

If I set up the view-only wallet to scan just 1000 subaddresses

@will-yjn How are you setting up the wallet to scan only a subset of subaddresses?

@will-yjn
Copy link
Author

@woodser sorry. I thought it defaults to 1000 subaddress. I just check the doc: https://monerodocs.org/interacting/monero-wallet-cli-reference/#performance. By default it checks 50:200 which are 10,000 subaddresses.

So first I set up the view wallet without specifying --subaddress-lookahead. Second I set up the view wallet specifying --subaddress-lookahead 1:1000000. In the second way, the view wallet correctly scans the deposit at the subaddress with index 1:181,162, thus exports an output hex much longer than the the first way would. But this code snippet:

image

...will throw the error:

TypeError: (intermediate value) is not iterable

To reproduce the bug: generate a subaddress at index 1:181162, make a deposit to the primary address and this subaddress, set up the view wallet with --subaddress-lookahead 1:1000000, view-wallet scans the chain, export the output hex from the view wallet, and then export the key images with the code above.

@woodser
Copy link
Owner

woodser commented Aug 24, 2022

The full wallet doesn't currently support creating new wallets with subaddress lookahead which also caused errors in my tests with big subaddress indices.

I'll add support for the next release. In the meantime, you could either create the offline wallet using monero-wallet-cli and --subaddress-lookahead then open with monero-javascript, or if you build the wasm files yourself, insert wallet->m_w2->set_subaddress_lookahead(1, 1000000); before wallet->m_w2->generate() lines in monero_wallet_full.cpp to create wallets with a specific subaddress lookahead.

I confirmed this works with deposits at index 1:181162.

@woodser
Copy link
Owner

woodser commented Aug 29, 2022

Subaddress lookahead is now supported in the v0.7.4 release when creating full wallets. For example:

const wallet = await monerojs.createWalletFull({
        networkType: MoneroNetworkType.MAINNET,
        password: "...",
        primaryAddress: "...",
        privateSpendKey: "...",
        privateViewKey: "...",
        accountLookahead: 1,
        subaddressLookahead: 1000000
});

@will-yjn
Copy link
Author

will-yjn commented Sep 1, 2022

It worked. But once I set subaddressLookahead to be above 10,000. It becomes very slow. And it is linearly increasing in time. Is there a way to optimize the time cost?

@woodser
Copy link
Owner

woodser commented Sep 1, 2022

Not to my knowledge. The wasm is generally underpowered because it's running single threaded. There's an effort to support multithreading which could help a lot. Otherwise it's dependent on the wallet implementation from monero-project.

@will-yjn
Copy link
Author

will-yjn commented Sep 1, 2022

Cool. I'm going to close this issue and follow that thread.

@will-yjn will-yjn closed this as completed Sep 1, 2022
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

No branches or pull requests

2 participants