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

Remove refresh from call from import_multisig #9312

Closed
woodser opened this issue Apr 29, 2024 · 3 comments
Closed

Remove refresh from call from import_multisig #9312

woodser opened this issue Apr 29, 2024 · 3 comments
Labels

Comments

@woodser
Copy link
Contributor

woodser commented Apr 29, 2024

This issue requests removing the refresh call from wallet2's import_multisig if possible:

refresh(false);

Refreshing is a relatively expensive operation which can take considerable time, especially over tor, and the wallet should be refreshing periodically anyway, so it could make most sense to remove this call.

Otherwise I observe this call hanging on the tor network and even timing out when it may not be necessary at all.

For comparison, refresh isn't called when key images are imported, etc.

@jeffro256
Copy link
Contributor

To keep behavior consistent, we could make an argument do_refresh_after and default it to true.

@woodser
Copy link
Contributor Author

woodser commented Apr 30, 2024

That sounds ok; the important part is to change the default behavior in monero-wallet-rpc's import_multisig call so clients do not have to refresh in order to use this function.

That would lead to the question if the internal default should be false too, considering very few services are hooking into this call directly. But that'd be an internal implementation detail, as long as clients do not need to refresh.

@selsta selsta added the wallet label May 3, 2024
@woodser
Copy link
Contributor Author

woodser commented May 4, 2024

Based on testing, the refresh is necessary or the wallet cannot create txs.

@woodser woodser closed this as completed May 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants