threefoldtech / tfgrid-sdk-ts

Apache License 2.0
4 stars 8 forks source link

Ensure compatibility with TFChain 2.9 release #3340

Open sameh-farouk opened 2 weeks ago

sameh-farouk commented 2 weeks ago

Which package/s are you suggesting this feature for?

No response

Is your feature request related to a problem? Please describe

Ensure compatibility with TFChain 2.9 release / TFChain runtime v152

Describe the solution you'd like

For better context PR: https://github.com/threefoldtech/tfchain/pull/992

Relevant SDK Adjustments Needed:

Fund Reservations: The system now employs a more reliable reservation method, replacing the previous use of locks/freezes.

Balance Handling: Payments on hold are no longer considered part of the free balance. The total balance should now reflect both the reserved and free portions. Previously, the free balance was considered the total balance, with amounts reserved for future payments subtracted from it. The new approach aligns with the balances pallet terminology:

Contract Payment Tracking: The ContractLock has been replaced with ContractPaymentState to better track and manage the states of contract payments, particularly during grace periods and when dealing with overdue payments. https://github.com/threefoldtech/tfchain/blob/622e7d255b9c76203fba8a316b84b280b59fbba7/substrate-node/pallets/pallet-smart-contract/src/types.rs#L219-L224

https://github.com/threefoldtech/tfchain/blob/83ccaa45d86076f6730d3611ce0c826c69dce649/substrate-node/pallets/pallet-smart-contract/src/types.rs#L236-L243

Dashboard Restore Contracts Feature: After the update to the new runtime, the dashboard's "restore contracts" feature will no longer be usable. To correctly reinstate this functionality, the following steps are needed:

Update: Possibly less relevant changes:: Event Emission Changes: The events emitted from the billing process have been updated, and several changes have been made:

New Errors:

sameh-farouk commented 2 weeks ago

@AhmedHanafy725 A last-minute change that is worth highlighting. https://github.com/threefoldtech/tfchain/pull/992#issuecomment-2311451933

0oM4R commented 2 weeks ago

Will start with the balance changes and running the node, and will apply the contract lock changes tomorrow: will give it 4h for both.

0oM4R commented 2 weeks ago

Issue update: applied balance changes on dashboard

0oM4R commented 2 weeks ago

Issue Update: support getting overdue amount functions. I faced some issues about the overdue types and typegeneration; update generatetypes script and used Decimal as we bigint supported only after ES2020

Question: in dashboard, should we go with locked contract and locked amount or paused contract and overdue contract

time estimate update: will add 2h for playground edits

sameh-farouk commented 2 weeks ago

Question: in dashboard, should we go with locked contract and locked amount or paused contract and overdue contract

For the first case, paused contract is okay, but I prefer suspended contract. Both terms are suitable.

For the second case, locked amount and overdue contract refer to different things. If you need to reference the amount that needs to be settled before contracts can resume, you may use the term overdue amount. Alternatively, overdraft amount can be used, or maybe just overdraft as we refer to it on tfchain.

On the other hand, the locked amount is now referred to as the reserved amount and represents the amount billed but not fully processed (pending transfer to beneficiaries).

0oM4R commented 2 weeks ago

Issue update: applied the changes on dashboard functions, but kept the old UI and names. I couldn't test the pr will keep it as drafted till we have chain deployed to test the logic.

0oM4R commented 2 weeks ago

moved to blocked on chain deployment

sameh-farouk commented 1 week ago

Any reason for keeping old names?

I understand the need for consistency to avoid confusing users, but this should not prevent us from correcting or improving some UI terms.

Referring to the amount needed to be settled as a locked amount is not correct. It's the other way around; the locked amount is the amount already settled/reserved. Suspended or paused contracts term are much clearer than locked contracts.

sameh-farouk commented 1 week ago

Just a friendly reminder, we should conduct tests locally whenever possible. Here are the steps to follow:

I understand the temptation to wait for the devnet deployment, but I want to highlight your options here. I believe it would be a valuable experience to have as well (testing using local tfchain instance).

0oM4R commented 1 week ago

Any reason for keeping old names?

I understand the need for consistency to avoid confusing users, but this should not prevent us from correcting or improving some UI terms.

Referring to the amount needed to be settled as a locked amount is not correct. It's the other way around; the locked amount is the amount already settled/reserved. Suspended or paused contracts term are much clearer than locked contracts.

I'm leaving it as is until we talk to @AhmedHanafy725 . I will check with him as soon as possible and apply the changes.

0oM4R commented 1 week ago

Just a friendly reminder, we should conduct tests locally whenever possible. Here are the steps to follow:

  • Launch a container using the latest tfchain image 2.9.0
  • Connect a local instance of the dashboard to the local tfchain node
  • over Polkadot.js, Agree to the terms and conditions, and create a twin
  • Create a name contract (as it is easier to create and doesn't require a node/farm setup)
  • After that, you can test any scenario involving billing, balances, or the grace period.

I understand the temptation to wait for the devnet deployment, but I want to highlight your options here. I believe it would be a valuable experience to have as well (testing using local tfchain instance).

grid client is using graphql to generate the consumption rate, and it is used to get the overdue amount. already tested creating name contract and getting the payment state but I couldn't calculate the overdue amount to test resuming graceperiod contracts. also the dashboard needs graphql to list the contracts, so i can't test the flow in the current situation. if you have any suggestion about testing please let me know

sameh-farouk commented 1 week ago

@0oM4R You could run a local Graphql instance that listens to the local TFChain.

0oM4R commented 1 week ago

@0oM4R You could run a local Graphql instance that listens to the local TFChain.

As discussed with Thabet will wait for the chain deployment

sameh-farouk commented 1 week ago

Runtime was updated on devnet

0oM4R commented 1 week ago

Issue update: Applied pr comments, and working on writing the new contract coast calculation, this also needs some logic to get the required parameters in the calculation function. will update the estimate time to be more 3 hours for implementation only

0oM4R commented 1 week ago

Issue update:

cost calculation is almost done, need to handle musd price and the node contract on rented node case.

todo: fix broken features related to grace period in dashboard, after complete clients changes

0oM4R commented 1 week ago

Issue update: client side changes almost complete, need to verify some calculations, will implement the dashboard changes tmw

0oM4R commented 1 week ago

Issue update: we need to refactor the flow of node contract on rented node to add the overdraft and nu and ipv4 costs -- Blocked on the calculations, i don't understand the current calculations, the returned amount is not clear on which unit

0oM4R commented 1 week ago

Issue update: prices now should be correct; the node contracts with ipv4 cost will be added to its rent contract also the un-billed NU. will verify the domain name tmw.

dashboard now calling the new methods, need to implement the new flow of node contracts on the rent contract only;

0oM4R commented 6 days ago

unique name calculation looks fine. To do:

will update the time estimation to more 8 hours; for UI changes, test, clean up the code, and prepare the pr

0oM4R commented 5 days ago

Issue update : all changes applied, will give it another test phase and prepare the PR

0oM4R commented 5 days ago

Issue update: while testing faced a edge case, when rent contract got billed and the node contract got crated after that, and got billed before the next billling cycle of the rent contract; in this case we have node contract on grace period, and its rent contract is on created state. work completed:

todo: handle the mentioned case in UI, and test it. will take about 3h note: testing cycle is very long, creating contracts and move them to grace period, and in some cases you have to wait for the next billing cycle.

0oM4R commented 4 days ago

For rephrasing will do in separate issue #3400