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 custom operation plugin check from custom api creation #2033

Merged
merged 3 commits into from Oct 29, 2019

Conversation

oxarbitrage
Copy link
Member

For #2032

@pmconrad
Copy link
Contributor

Actually the check is correct (and should even be added for other APIs that require plugins as well, see some other issue). Note that even without the check the error will occur if a node doesn't enable the API, which would be pointless if the plugin is not active.
I agree with @abitmore that the client must handle the case correctly, i. e. libraries/wallet in this case.

@oxarbitrage
Copy link
Member Author

ok, trying something else.

@oxarbitrage
Copy link
Member Author

New approach at 860fe96

@oxarbitrage oxarbitrage changed the title remove custom operation plugin check from customn api creation remove custom operation plugin check from custom api creation Oct 25, 2019
}
catch(const fc::exception& e)
{
wlog("Custom operations plugin is not loaded.");
Copy link
Contributor

Choose a reason for hiding this comment

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

Better: Custom operations API is not active on server.
What happens in this case if get_account_storage is invoked?

Copy link
Member Author

Choose a reason for hiding this comment

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

Better: Custom operations API is not active on server.

Changed.

What happens in this case if get_account_storage is invoked?

Crash, added try to call, thanks :)

Both changes at f6738ec

Note: No need to add try at account_store_map as it does not use the API. custom operations can be created without the plugin or the api being enabled, not possible to read without them.

@abitmore abitmore added this to In development in Protocol Upgrade Release (4.0.0) via automation Oct 25, 2019
Copy link
Contributor

@pmconrad pmconrad left a comment

Choose a reason for hiding this comment

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

Thanks!

@jmjatlanta jmjatlanta merged commit b70d7f2 into release Oct 29, 2019
Protocol Upgrade Release (4.0.0) automation moved this from In development to Done Oct 29, 2019
@jmjatlanta jmjatlanta deleted the custom_plugin_wallet_error branch October 29, 2019 17:48
@pmconrad pmconrad mentioned this pull request Oct 30, 2019
17 tasks
@abitmore abitmore mentioned this pull request Feb 25, 2020
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

4 participants