Skip to content

Increase the CPU execution time limit in the TVM of the triggerconstantcontract API #6288

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

Open
yanghang8612 opened this issue Apr 15, 2025 · 17 comments
Assignees

Comments

@yanghang8612
Copy link
Contributor

Background

In the TRON network, the triggerconstantcontract interface is widely used for off-chain simulation calls (dry-runs). However, this interface is constrained by the execution time limit of the TVM (mainnet, 80ms). When simulating complex contract logic or intensive state access, this can lead to out_of_time exceptions, affecting the reliability of development tools and analysis services.

While running the FullNode with the --debug flag can remove the TVM timeout restriction for triggerconstantcontract calls, this setting also removes the timeout limit for transactions within blocks during normal block processing. This can introduce potential instability or denial-of-service risks to the node, making it unsuitable for production environments.

Related issues: #6266

Rationale

Why should this feature exist?

Allowing users to configure a custom TVM timeout in dry-run calls would:

  • Improve the stability of off-chain analysis tools

  • Prevent false negatives for complex contracts

  • Reduce misleading error codes that confuse developers

What are the use-cases?

  • Data services performing bulk state queries via contract calls

  • Deep cross-contract call simulation

  • Simulating complex logic (e.g., loops or mappings) that exceed the default execution window

Specification

Introduce a new optional field in the triggerconstantcontract request body:

{
    ...
    "timeout_ms": 200
}

timeout_ms: Allows the caller to specify a custom timeout (e.g., 200ms). Default behavior remains unchanged (e.g., 80ms).

A maximum cap (e.g., 500ms) should be enforced to prevent abuse.

Test Specification

TODO

Scope Of Impact

  • Backward-compatible API change

  • TVM executor must support receiving the custom timeout

  • SDKs and documentation need to be updated accordingly

Implementation

TODO

Do you have ideas regarding the implementation of this feature?

  • Extend the interface parser to accept and pass the timeout_ms field

  • Update the TVM executor to handle the custom timeout logic

  • Enforce a reasonable upper limit to avoid long blocking operations

Are you willing to implement this feature?

Yeap, happy to do it.

@Sunny6889
Copy link

@yanghang8612 Very good proposal. Regarding your previews comment here:

Therefore, in the debug mode, when the received block contains a transaction whose execution result is OUT_OF_TIME, the local execution result of the transaction will be something other than OUT_OF_TIME (because the TVM will not check for timeout), which is inconsistent with the result of the execution of the transaction within the block. This will result in a different result code exception and the local node will stop synchronising.

I wonder what the upper limit of timeout_ms would be to greatly reduce the chance of ^ happens? Actually as my understanding, as long as timeout_ms > 80ms, it may happens. So actually, we cannot avoid the denial-of-service happens.

@waynercheung
Copy link
Contributor

@yanghang8612 @Sunny6889 Hi, I found that the eth_call is implemented the same as triggerconstantcontract, do you want to add the timeout_ms parameter to eth_call? or just add the parameter to triggerconstantcontract?

@Sunny6889
Copy link

Sorry previously I thought about you add it to configuration file. If we implement the timeout setting in the request body instead, it would enforce a uniformly agreed-upon timeout across the system. This approach would require a new committee proposal for implementation, as direct modification could potentially cause blockchain forks.

@yanghang8612
Copy link
Contributor Author

@yanghang8612 Very good proposal. Regarding your previews comment here:

Therefore, in the debug mode, when the received block contains a transaction whose execution result is OUT_OF_TIME, the local execution result of the transaction will be something other than OUT_OF_TIME (because the TVM will not check for timeout), which is inconsistent with the result of the execution of the transaction within the block. This will result in a different result code exception and the local node will stop synchronising.

I wonder what the upper limit of timeout_ms would be to greatly reduce the chance of ^ happens? Actually as my understanding, as long as timeout_ms > 80ms, it may happens. So actually, we cannot avoid the denial-of-service happens.

Yes, perhaps we can limit the number of requests that can be processed at the same time with a timeout_ms parameter of more than 80ms through node configuration. It's not a very common requirement after all.

@yanghang8612 yanghang8612 changed the title Extend the triggerconstantcontract interface so that it can override the TVM timeout limit Increase the CPU execution time limit in the TVM of the triggerconstantcontract API Apr 15, 2025
@yanghang8612
Copy link
Contributor Author

@yanghang8612 @Sunny6889 Hi, I found that the eth_call is implemented the same as triggerconstantcontract, do you want to add the timeout_ms parameter to eth_call? or just add the parameter to triggerconstantcontract?

Sure, but what I'm thinking about is whether adding parameters to the eth_call is going to be difficult in terms of integration for Ethereum developers.

@onm798
Copy link

onm798 commented Apr 17, 2025

@yanghang8612 @Sunny6889 Hi, I found that the eth_call is implemented the same as triggerconstantcontract, do you want to add the timeout_ms parameter to eth_call? or just add the parameter to triggerconstantcontract?

Sure, but what I'm thinking about is whether adding parameters to the eth_call is going to be difficult in terms of integration for Ethereum developers.

Adding an new parameter in eth_call might make it different from the Ethereum json-rpc standard. As this feature would enable the TVM executor to adopt a custom timeout logic, maybe it's possible to add a node-level-config timeout limit for tvm execution from eth_call.

@yanghang8612
Copy link
Contributor Author

@yanghang8612 @Sunny6889 Hi, I found that the eth_call is implemented the same as triggerconstantcontract, do you want to add the timeout_ms parameter to eth_call? or just add the parameter to triggerconstantcontract?

Sure, but what I'm thinking about is whether adding parameters to the eth_call is going to be difficult in terms of integration for Ethereum developers.

Adding an new parameter in eth_call might make it different from the Ethereum json-rpc standard. As this feature would enable the TVM executor to adopt a custom timeout logic, maybe it's possible to add a node-level-config timeout limit for tvm execution from eth_call.

Good advice!

Another thing I would like to share is that if the parameter is set to a larger value like 200ms, then there will be an extra burden on eth_call and is it possible that this could generate a dos attack vector. Therefore it is important to limit the valid range of this parameter and also to carefully evaluate if some limitations can be imposed on the eth_call interface itself to minimize potential problems.

@abc-x-t
Copy link

abc-x-t commented May 29, 2025

Adding an new parameter in eth_call might make it different from the Ethereum json-rpc standard. As this feature would enable the TVM executor to adopt a custom timeout logic, maybe it's possible to add a node-level-config timeout limit for tvm execution from eth_call.

Good advice!

Another thing I would like to share is that if the parameter is set to a larger value like 200ms, then there will be an extra burden on eth_call and is it possible that this could generate a dos attack vector. Therefore it is important to limit the valid range of this parameter and also to carefully evaluate if some limitations can be imposed on the eth_call interface itself to minimize potential problems.

Hi, I think add a node-level-config is a better way, who want to test this situation they could modify the config of the nodes to support a custom time limitation.
Maybe more things need to be considered are: whether need to tell the caller how long exactly this transaction used, whether can this transaction be executed successfully on the mainnet or something like these that.

@317787106
Copy link
Contributor

@abc-x-t I agree with you. Add request parameter out_of_time to api triggerconstantcontract is not a good idea. User should not control the logic of server.

@Sunny6889
Copy link

@abc-x-t @317787106 One risk about adding it to a node-level-config would be it may cause insistence among SRs.

@lxcmyf
Copy link
Contributor

lxcmyf commented May 29, 2025

@abc-x-t I agree with you. Add request parameter out_of_time to api triggerconstantcontract is not a good idea. User should not control the logic of server.

If a transactional contract transaction calls triggerconstantcontract internally, different configuration nodes may have different results for processing the transaction. Based on this, it may not be suitable to place it in node configuration.

@yanghang8612
Copy link
Contributor Author

@abc-x-t @317787106 One risk about adding it to a node-level-config would be it may cause insistence among SRs.

Consensus transaction execution time is inherently an impossible task because of the differences in machine performance between SR nodes and the fact that even the exact same machine performance is not likely to produce the exact same processing market for the same transaction.

@yanghang8612
Copy link
Contributor Author

@abc-x-t I agree with you. Add request parameter out_of_time to api triggerconstantcontract is not a good idea. User should not control the logic of server.

I don't agree with what you call controlling the execution logic of the service. If that's considered, wouldn't the different interface parameters also be considered as controlling the execution logic of the service? As you phrased it, you implicitly think this is a dangerous thing to do, so you have to give specific counter-examples to prove your point.

@abc-x-t
Copy link

abc-x-t commented May 29, 2025

@abc-x-t @317787106 One risk about adding it to a node-level-config would be it may cause insistence among SRs.

@Sunny6889 triggerconstantcontract is only used for testing and will not be broadcast to the chain. As long as SRs properly manage the API, there should be no performance impact.

@yanghang8612
Copy link
Contributor Author

Essentially, whether it's at the node configuration level, or by adding parameters at the interface level, its possible to potentially introduce a ddos attack. For example, unreasonably large node configuration parameters or concurrent interface access with large parameter values.

@yanghang8612
Copy link
Contributor Author

So we also mentioned in the previous discussion that for this type of access, regardless of the form of the implementation, it is important to strictly limit the frequency of accesses to this type of call, and to carefully design the valid ranges of specific parameters.

@yanghang8612
Copy link
Contributor Author

As per previous design conventions, perhaps we could add a node configuration switch for nodes to enable this new feature.

Or in order to avoid differences in understanding while retaining interface compatibility, perhaps we could accomplish the purpose by adding a new interface, adding a /wallet/simulatesmartcontract or something like that? Or even implement the interface in asynchronous mode, where the call returns a task id and the result of the execution needs to be retrieved via /wallet/getsimulationresult?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: To Do
Development

No branches or pull requests

7 participants