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

CNS-334: Subscription fixes, tests, and CLI #351

Merged
merged 11 commits into from
Mar 15, 2023

Conversation

orenl-lava
Copy link
Contributor

@orenl-lava orenl-lava commented Mar 9, 2023

  1. Fix bug in subscription nextMonth() on Jan 30,31 (spilled over to March)
  2. Add unit tests for calculation of ExpiryTime (regular and yearly plans)
  3. Add unit tests for calculation of plan cost (yearly with/without discount)
  4. Replace is_yearly with duration, all plans are for 1 month (remove plan Duration)
  5. Rename CLI from tx subscription subscribe to tx subscription buy
  6. Improve CLI for (tx) buy and (query) current-subscription
  7. Make the buy msg args "consumer", "duration" optional (defaults: "creator", "1" respectively).

@orenl-lava orenl-lava requested review from Yaroms and omerlavanet March 9, 2023 19:56
@orenl-lava orenl-lava force-pushed the CNS-334-subscription-fixes-tests-and-cli branch 2 times, most recently from e641114 to 43489bd Compare March 10, 2023 03:50
proto/plans/plan.proto Outdated Show resolved Hide resolved
x/subscription/client/cli/query_current_subscription.go Outdated Show resolved Hide resolved
x/subscription/client/cli/tx_buy.go Outdated Show resolved Hide resolved
Yaroms
Yaroms previously approved these changes Mar 12, 2023
x/subscription/client/cli/tx_buy.go Outdated Show resolved Hide resolved
x/subscription/keeper/subscription.go Outdated Show resolved Hide resolved
x/subscription/keeper/subscription.go Outdated Show resolved Hide resolved
x/subscription/types/subscription.go Show resolved Hide resolved
omerlavanet
omerlavanet previously approved these changes Mar 15, 2023
AdvanceBlock(): use ctx.WithBlockTime() to update the ctx's block time.
InitAllKeppers(): use AdvanceBlock instead of NewBlock() and get a new ctx.
The result of NextMonth() on Jan 30,31 was Mar 1 or 2.
Fix it to be the end of February.
During tx-subscribe we calculate the subscription expiry time. To ensure to the
start-time is deterministic, use the current block timestamp.
subscribe:
  lavad tx subscription subscribe PLAN [CONSUMER] [IS_YEARLY] [flags]

- PLAN is mandatory
- CONSUMER is optional (default is creator's address)
- IS_YEARLY is optional (default is false)

current-subscription:
  lavad query subscription current-subscription [CONSUMER]

- CONSUMER is optional (default is creator's address)
Not needed because all state changes are reverted anyhow upon error.
Rename the msg 'subscribe' to 'buy.
Replace `is_yearly` with `duration` (in months)
Plans are always for 1 month.
The `tx subscription buy` message indicates duration (in months).
For `query current-subscription` arg is mandatory (back).
For `tx buy` indicate optional args in usage string.
Mark removed field as reserved and restore original identifiers.
@orenl-lava orenl-lava force-pushed the CNS-334-subscription-fixes-tests-and-cli branch from c8f3ff9 to d6bd5ff Compare March 15, 2023 17:02
@omerlavanet omerlavanet merged commit f2ccffe into main Mar 15, 2023
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