💭 Change 🧠 Logic limits type in Protobuf

[ccamel]

Context

The Limits protobuf structure is designed to specify various constraints in the logic module, including computational power (gas), exeuted program size, result count, and user output size. These limits are crucial for maintaining system performance, security, and resource allocation efficiency.

Currently, all fields (max_gas, max_size, max_result_count, max_user_output_size) uses a custom type, cosmossdk.io/math.Uint, to handle these constraints, offering potentially unlimited range for specifying these values. However, using such a type seems unnecessary considering the range of values these parameters typically hold.

The goal of this conversation is to reflect on the proposal and gather feedback from the community to identify the most appropriate type choices for the interface.

Proposition

This proposal suggests replacing the string type annotated to use cosmossdk.io/math.Uint with a regular uint type for the fields max_size, max_result_count, and max_user_output_size within the Limits protobuf definition.

This change is based on the assessment that the range provided by the uint64 type is more than sufficient for specifying the maximum size, result count, and user output size in practical scenarios in the protocol. The max_gas field will retain its current implementation using cosmossdk.io/math.Uint due to its association with currency values, which may require a broader range than what uint can offer.

Changes to be made

• max_size from string to uint64
• max_result_count from string to uint64
• max_user_output_size from string to uint64

Unchanged

• max_gas

Consequences

Positive:

• Simplicity and clarity: Using a native uint type simplifies the protobuf definition and makes it more straightforward for developers to understand and use improving the DX.
• Performance: Native types are (generally) more efficient in terms of serialization and deserialization.

Negative:

• Migration Effort: A migration of the module should be considered to ensure that the storage of these values is still functional.

Neutral:

• Serialization Impact for Clients: Since uint64 values are serialized as strings, the change in type should have a neutral impact on clients utilizing the JSON or YAML formats of the interface.

Request for Community Feedback

This proposal seeks feedback from the community on the following points:

• Impact assessment: Insights on the practical impact of the proposed range limitations
• Client migration concerns: Any concerns or challenges anticipated with migrating existing systems to comply with the proposed changes.
• Alternative suggestions: Recommendations for alternative approaches that balance type safety, performance, and flexibility.

[ccamel]

@amimart @bdeneux

[ccamel]

@amimart, @bdeneux What do you think about this one, guys?

[amimart]

I totally approve the approach, we shall use the most appropriate types.

It's also worth to mention that those limits have a bit changed since this initiative:

• The max_gas limit has been removed through Fix/logic gas limit axoned#708;
• The max_variables limit has been added through 🧠 Use our own axone/prolog fork in logic module axoned#703;

To sum up the needed changes if going forward with it, we'll change the following fields:

• max_gas: from string tu uint64;
• max_size: from string tu uint64;
• max_result_count: from string tu uint64;
• max_user_output_size: from string tu uint64;
• max_variables: from string tu uint64;

As this is a breaking change in the API, the question of supporting as deprecated the previous design should be addressed. From my point of view we are on testnet and I consider acceptable to break things, especially things not used by client applications like this. What do you think?

[ccamel]

I totally agree. Let's do it!

[ccamel]

axone-protocol/axoned#784

[ccamel]

All done.