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

Make cw721-base queries public #399

Merged
merged 1 commit into from
Sep 2, 2021
Merged

Make cw721-base queries public #399

merged 1 commit into from
Sep 2, 2021

Conversation

maurolacy
Copy link
Contributor

@maurolacy maurolacy commented Sep 1, 2021

On behalf of @shanev.

Closes / replaces #397.

@maurolacy
Copy link
Contributor Author

maurolacy commented Sep 1, 2021

cw-plus contracts are more or less split in that they make these internal query functions public:

$ grep "fn query_" contracts/*/src/contract.rs | grep ":pub" | wc -l
28
$ grep "fn query_" contracts/*/src/contract.rs | grep -v ":pub" | wc -l
26
$ 

So, I would say, this is perhaps not best practice, but it's OK / allowed.

@shanev
Copy link
Contributor

shanev commented Sep 1, 2021

@maurolacy What's the case against making all query functions public?

@maurolacy
Copy link
Contributor Author

@maurolacy What's the case against making all query functions public?

None, really. It's just that it's supposed these functions must be called through the query handler. But as you said, it's pretty convenient for tests. And making them public is not a risk, because they cannot be called directly in a production environment anyway.

@maurolacy maurolacy merged commit 7dded44 into main Sep 2, 2021
@maurolacy maurolacy deleted the cw721-public-queries branch September 2, 2021 05:24
@ethanfrey
Copy link
Member

@maurolacy What's the case against making all query functions public?

None, really. It's just that it's supposed these functions must be called through the query handler. But as you said, it's pretty convenient for tests. And making them public is not a risk, because they cannot be called directly in a production environment anyway.

Nice fix.

I'm fine to make them all public. I was trying not to expose everything but you do need all the execute* and query* functions to be public to extend a contract, so why not do that everywhere just in case someone wants to extend it?

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

Successfully merging this pull request may close these issues.

3 participants