chain release2.9 compatibility Grace Period and overdue #3387
Conversation
- a method to get NU billing information - a method to get contract payment state - a method to get used resources by the node contract - some related interfaces
- use proxy url in unlockMycontracts - refactor unlockContracts to accept array of contracts - add function to unlock contracts by their ids
- add function that takes only contract to unlock it - add calculate overdue functions
- add contracts on rented node to the rent node overdue
it is storing the rent contract on a map by the node id, and ignore the unwanted calculations
unlock dialog now will check if the node contract on rented node or not; if so will retrive the rent contract cost and store the and will avoid duplication
…d with selected contracts
| state: [ContractState.GracePeriod], | ||
| numberOfPublicIps: 1, | ||
| }); | ||
|
|
There was a problem hiding this comment.
Can we handle the case where there are no returned contracts and return early?
| if (!contract.details.number_of_public_ips) return 0; | ||
|
|
||
| const ipPrice = (await this.client.pricingPolicies.get({ id: 1 })).ipu.value / 10 ** 7; | ||
| return ipPrice * 30 * 24; |
There was a problem hiding this comment.
Why not use the number of public IPs in the contract instead?
FYI, from the tfchain side, I can create a contract that reserves more than one public IP. Since it is a valid case, despite some client limitations or restrictions, it should be handled when calculating the contract costs as it may be created with different clients.
| /** Other contract types need the node information */ | ||
| const nodeDetails = await proxy.nodes.byId(contract.details.nodeId); | ||
|
|
||
| const certified = nodeDetails.certificationType == CertificationType.Certified ? true : false; |
There was a problem hiding this comment.
Do you need the ternary operator here?
The condition nodeDetails.certificationType == CertificationType.Certified already evaluates to true or false.
Also, it is better to use === to avoid unexpected behavior caused by implicit type conversion.
|
|
||
| // cost of the current billing period with the mentioned allowance time | ||
| const totalPeriodCost = contractCostTFT.times(totalPeriodTime); | ||
| const overdue = totalOverDraft.add(unbilledNU); |
There was a problem hiding this comment.
As previously explained in my other comment, You can't add them directly.
| const { standardOverdraft, additionalOverdraft, lastUpdatedSeconds } = | ||
| await this.client.contracts.getContractPaymentState(contractInfo.contract_id); | ||
|
|
||
| /**Calculate the elapsed seconds since last pilling*/ |
There was a problem hiding this comment.
There is a small typo here "pilling" -> "billing"
| const overdue = totalOverDraft.add(unbilledNU); | ||
|
|
||
| //convert to TFT | ||
| const OverdueTFT = overdue.div(10 ** 7); |
There was a problem hiding this comment.
OverdueTFT name should follow camelCase convention.
| if (nodeDetails.rented) { | ||
| if (!contract.details.number_of_public_ips) return 0; | ||
|
|
||
| const ipPrice = (await this.client.pricingPolicies.get({ id: 1 })).ipu.value / 10 ** 7; |
There was a problem hiding this comment.
I would avoid using magic numbers like 10 ** 7 and define it as constant TFT_CONVERSION_FACTOR (maybe in the client) then use it everywhere else for clarity and maintainability.
There was a problem hiding this comment.
add for the current file, will open an issue to make it global value for the grid client
|
|
||
| // cost of the current billing period with the mentioned allowance time | ||
| const totalPeriodCost = contractCostTFT.times(totalPeriodTime); | ||
| const overdue = totalOverDraft.add(unbilledNU); |
There was a problem hiding this comment.
Another point to consider regarding unbilled NU is that it may be eligible for a discount or subject to an added premium:
- A discount may apply based on the user's balance.
- A premium is added when a certified node is used.
This has already been handled for overdraft stored on the contract payment state, as well as the cost of the contract retrieved from the calc module.
separate node contracts on rented node cost calculations to multiple functions create constants with numbers repeatedly used in the calculations enhance docstring
|
@0oM4R I attempted a quick test, but it didn't go well.
|
|
Another point, it seems you haven't incorporated the changes discussed in the tg group yet.
|
…into development_chain_release2.9
there is a separate issue for it #3400 |
|
works fine with me but we should handle the zero tft case in the ui
|
Sorry, My bad. |
|
Calculations looks much better now. |
…rting cost per second
|
Test report: Rent contract without any node contracts on it
the allowance window in the next scenarios is 60 seconds |
skip the node contracts on rented nodes as it already calculated in rent contract
we were ignoring the elapsed seconds for node contracts and we assume that it is the same as rent contract
Description
We have new accurate way to resume/unlock contracts on tfchain2.9, this pull request is for replace the old way as it is no longer supported.
General idea:
Now the amount required to resume the contract is described as Overdue.
The overdue amount basically is the sum of three parts:
Changes
Please node: you may notice that we have multiple functions that generally do the same thing but require different arguments.
for example: we need some information that is already in the contract type,
so instead of requesting the contract in side the function we are passing it.
but in some cases we only have the contract id? we provide a function that take the contract id and then request the contract then pass it to the inner function.
TFChain client
Grid client:
Dashboard
support new functions in grid client, refactor, and remove the deprecated gird client functions.
we have some changes in the flow:
If the user select node contracts, and some of them are on a rent contract, we will show the rent contract id with the selected contracts
if use click on grace period chip of node contract with ipv4 on rented node the content of the dialog depends on the rent contract state:
if the rent contract state is
Created: will get the node contract overdue, and user will be able to unlock the contract.If on
Grace period: will show a message that you will have to check the rent contract and unlock it.fix loading spinner in loading locked amount details:
Related Issues
#3340
#3364
Tested Scenarios
show contract/s cost : this will be nearly from pricing calculator if the contract got billed right before moving to the grace period as in this case overdue will be small for example, name contract cost should be about
0.025tftsselect node contracts on rented node that its contract on grace period,
unlock all contracts associated with rent contract: in this case to verify that the node contract got billed, use
smarContractModule/contractPaymentStatecall, the additional and standred overdraft should be 0Unlock single or multiple contracts
in any case we should'n have a contract got billed twice: you can add log line to log the ids in batchUnlock contracts
Tips:
To send contract to a grace period, you need to follow some steps
1- empty you walet, this can be done by polkadot UI
Balances/TransferAll2- get the contract Id, must be billable one like rent contract or node contract on shared node, name contract, and call
smarContractsModule/billcontractForBlockusing any other funded account.to send node contract with public ip on rented node to grace period and keep it's rent contract is on created state
Documentation PR
For UI changes, Please provide the Documetation PR on info_grid
Checklist