feat: third party service contract for pallet-smart-contract #522
Conversation
renauter
requested review from
DylanVerstraete,
brandonpille,
robvanmieghem and
LeeSmet
as code owners
renauter
removed request for
robvanmieghem,
brandonpille,
DylanVerstraete and
LeeSmet
| // Billing time (window) is max 1h by design | ||
| // So extra time will not be billed | ||
| // It is the service responsability to bill on right frequency | ||
| let window = elapsed_seconds_since_last_bill.min(3600); |
There was a problem hiding this comment.
Maybe create a const variable for the 3600? So that if it is ever changed we can quickly change it.
There was a problem hiding this comment.
Or even make it a config parameter type?
There was a problem hiding this comment.
Or even make it a config parameter type?
I did the first option for now
Not sure if I see what you mean by the second option
There was a problem hiding this comment.
It's a parameter that you can set in the definition of the config of a pallet. Like here for example: https://github.com/threefoldtech/tfchain/blob/development/substrate-node/runtime/src/lib.rs#L381
There was a problem hiding this comment.
Let's see what @DylanVerstraete thinks. Not sure if it's needed.
There was a problem hiding this comment.
I added such config parameter type here
70bfb60
@brandonpille @DylanVerstraete please check if every thing is ok because changes can be critical ;)
There was a problem hiding this comment.
Looks great. Although maybe ServiceContractBillingFrequency would be a better name?
There was a problem hiding this comment.
maybe ServiceContractBillingFrequency would be a better name?
Not sure because it is used in other contexts
The idea is to characterize the amount of time (is seconds) price values are set for in pricing policy / billing context
maybe PriceReferencePeriod would be better?
There was a problem hiding this comment.
Oh I didn't know that. Then the name is fine how it is.
|
@renauter can you rebase and add an ADR please? |
@DylanVerstraete feel free to comment if need some rework |
|
Looks good to me, let's merge |
Gather changes from #513 and #495
Related to
See specs here:
#445 (comment)
To complete: