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

Check if node have active contracts before changing the power target to down #924

Open
sameh-farouk opened this issue Dec 31, 2023 · 4 comments · Fixed by #934
Open

Check if node have active contracts before changing the power target to down #924

sameh-farouk opened this issue Dec 31, 2023 · 4 comments · Fixed by #934
Assignees
Labels
type_feature New feature or request
Milestone

Comments

@sameh-farouk
Copy link
Member

Is your feature request related to a problem? Please describe

Currently, the farmer bot is responsible for verifying the active contracts before the node switches to low power mode, but this can cause data race issues.

Describe the solution you'd like

Check on change_power_target call if node have active contracts before changing the power target to Down

@renauter renauter self-assigned this Jan 9, 2024
@renauter renauter added the type_feature New feature or request label Jan 9, 2024
@renauter renauter added this to 3.13.x Jan 9, 2024
@renauter renauter moved this to In Progress in 3.13.x Jan 9, 2024
@renauter renauter added this to the 2.7.0 milestone Jan 9, 2024
@renauter renauter moved this from In Progress to Pending review in 3.13.x Jan 11, 2024
@github-project-automation github-project-automation bot moved this from Pending review to Done in 3.13.x Jan 24, 2024
@renauter renauter reopened this Jan 24, 2024
@renauter renauter moved this from Done to Pending Deployment in 3.13.x Jan 24, 2024
@sameh-farouk sameh-farouk moved this from Pending Deployment to In Verification in 3.13.x Jan 28, 2024
@renauter
Copy link
Collaborator

Already live on mainnet so it can be closed

@github-project-automation github-project-automation bot moved this from In Verification to Done in 3.13.x Mar 21, 2024
@scottyeager
Copy link

So, the side effect of this is that nodes with leftover (free) network contracts can no longer be powered down by farmerbot. We have quite a few network contracts hanging around for whatever reasons, and of course this also represents a pretty serious abuse vector.

The more logical thing to do is to actually check if any resources are reserved. I'm not sure if that's something we can comfortably do in the TF Chain runtime at each attempt to change power target to Down. If not, we need to come up with another approach.

@sameh-farouk
Copy link
Member Author

sameh-farouk commented Apr 17, 2024

@scottyeager IMO the issue you are refer to has no thing to do with this issue or its related feature.
Also, There is nothing called a network contract, there are node, rent and name contracts only. from the chain prospective a node contract is meant to reserve some resources on a node for some workload/s. hence, this node shouldn't be allowed to get into low power mode.
You can have one contract for vm and network. or a contract per workload. in latter case, the node contract related to the network will have no resources reserved regardless if there is a vm uses this network or no. so its hard to tell. even if we can sort to some kind of logic it won't be a proper solution to just allow the node to sleep. if the contract is useless for user its still reserving storage on chain (and the node).
We can push to add a fixed fee for contracts as mentiond on the other issue, but this progress should be tracked there not here and this one should be closed if all agree with me.

@scottyeager
Copy link

What I'm saying is that there are two direct results of the changes described in this issue:

  1. There are nodes that are doing no useful work that can no longer go standby
  2. Anyone can with a trivial amount of TFT stop every other node on the network from ever going standby

That's a big impact in exchange for fixing an issue that to me is not clearly described. So it's not clear to me what the overall impact is and whether that's a net positive, but I do think that side effects must be considered with any change.

I don't think there was any ambiguity regarding what I meant by "network contract". Indeed I wasn't referring to any formally defined entity, but rather the matter of fact that our deployment tools create node contracts that exist solely to hold network workloads. It turns out we have to talk about these quite a bit, especially because it's also a matter of fact that such contracts tend to get left behind when workloads are destroyed, so having a convenient way to refer to these is important.

Anyway, to me it's pretty simple: just let the node go to sleep if there aren't any resources reserved across all node contracts. It's true that network contracts are also used to include gateway nodes into user networks, so there could in theory be a node that has no resources reserved but is still doing useful work as a gateway. However, it would not make any sense for a farmer to configure a node as a gateway and then put it to sleep, so I think this possibility can be safely ignored.

As for the idea of fixed costs per contract, to me it's not really a complete solution for this issue. Placing a cost on contracts will likely just result in users getting billed a bit more but never noticing it in many circumstances since it will get "rolled in" with the cost of any contracts that are actually reserving capacity. Most users just won't notice an extra $.2 per month in charges any more than they already notice they have extra contracts hanging around. It is true though that this would help substantially with point 2.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type_feature New feature or request
Projects
No open projects
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants