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

[market-commons] Remove now() method from MarketCommonsPalletApi #635

Open
maltekliemann opened this issue May 18, 2022 · 2 comments
Open
Labels
p:low Low priority, resolution of this issue can wait t:enhancement The issue described an enhancement

Comments

@maltekliemann
Copy link
Contributor

The now() method of MarketCommonsPalletApi is used by prediction-markets to check for ending times of markets. Is there a reason why we refer to the market-commons pallet for the time, instead of just using the Timestamp of prediction-markets?

@maltekliemann maltekliemann added p:low Low priority, resolution of this issue can wait t:question The issue contains a question t:enhancement The issue described an enhancement labels May 18, 2022
@sea212
Copy link
Member

sea212 commented Jul 10, 2022

I think the idea Caio had when he created this was to avoid having another type in the Config trait of the prediction-market pallet, but rather using something we already have, the market-commons pallet, which is used almost everywhere and also needs timestamping functionality. Consequently it can expose the functionality and other pallets can use it. I have no strong opinion on that, however I see no real benefit of doing it this way and it might even be confusing. I think for the sake of clarity it would be better to remove it eventually, let's revisit this issue sometime in the future.

@sea212 sea212 removed the t:question The issue contains a question label Jul 10, 2022
@maltekliemann
Copy link
Contributor Author

I think we should remove this from the market-commons pallet.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
p:low Low priority, resolution of this issue can wait t:enhancement The issue described an enhancement
Projects
None yet
Development

No branches or pull requests

2 participants