-
Notifications
You must be signed in to change notification settings - Fork 15
Chronologic / Piper standard #16
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
base: master
Are you sure you want to change the base?
Conversation
* @return true if there is enough funds allowed to the contract to pay for subscription | ||
*/ | ||
|
||
function isFunded() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As we've discussed on Friday - this might be helpful wrapper but needs further discussion if it's really needed. Alternative is:
- Wallet gets
payment asset
type (e.g. ERC20). - Recognizes the contract type and calls
erc20contract.balanceOf(owner);
or/anderc20contract.allowance(owner, spender);
Thoughts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Even in the case of ETH, you probably want to store it in ERC20 compatible "vault" which makes this work by default.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will remove the burden of implementing this functionality outside the smart contract and provides the precise way to see the status of the subscription.
Underlying implementation will be as you suggested.
* @return true if cancellation succedded | ||
*/ | ||
|
||
function cancel() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Standard interface should not be limited to a "contract-per-subscription" model. I believe UUID should be provided at the least.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is more of the architectural choice to keep contract-per-subscription model. Given that subscription is going to hold the value, regardless whether ETH or tokens, we are striving to eliminate the honeypot problem.
* @return available funds | ||
*/ | ||
|
||
function availableFunds() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Above discussion applies to this function, too.
standards/chronologic-piper.md
Outdated
returns (uint256 funds); | ||
``` | ||
|
||
#### metadata |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is a good idea. Can we agree on this more polished version: https://github.com/EthereumOpenSubscriptions/standard/blob/99d833792ddd4d2a422fea66fb21379c7a2ef61a/standards/stef_and_luka-standard_proposal_draft.md#erc948metadata-interface? It was taken directly from ERC721 standard and I think they did a good job with the spec.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a very good point!
What are your thoughts on storing metadata as JSON key-value object?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like this idea. Removes some pollution caused by implementation specifics.
|
||
#### cancel | ||
|
||
Immediately cancels the subscription. Implementations should ensure that any unpaid subscription payments are paid to the provider as part of cancellation to ensure providers are able to let subscriptions fees fill up for arbitrary lengths of time, allowing them to reduce overhead from transaction costs. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we can ensure any payment executions since there are many different scenarios when the payment execution is not possible (eg. subscriber has insufficient funds).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can ensure unclaimed funds are paid if there are sufficient funds though. If there are not sufficient funds, perhaps there is a debt pattern that we can employ here. It may be out of scope for the standard but could be outlined as a suggested option.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would suggest leaving this for the provider / processor to handle. For instance, they could postpone a payment N-times until finally cancelling the subscription.
Subscriptions in fiat for example also cannot ensure the actual payment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree that's a little bit tricky. I was thinking to combine this with isFunded
and/or availableFunds
function described above.
So the scenario from the provider perspective is:
- provider monitors isFunded/availableFunds - this tells current funding status
- based on that information we can make a decision wether continoue the service (as @captnseagraves mentioned, provide the debt) or cancel
Based on Piper proposal, extended by monitoring functions and metadata. Closes #7.