search

cosmos / cosmos-sdk

:chains: A Framework for Building High Value Public Blockchains :sparkles:
https://cosmos.network/
Apache License 2.0
6.09k stars 3.52k forks source link

[Feature]: make timeout_height time based #20658

Open tac0turtle opened 1 month ago

tac0turtle commented 1 month ago

Summary

Dydx recently brought up wanting to use time instead of block height for unordered txs. This would need a new field in the TX, which is the opposite direction we wanted to go with making the tx simpler.

Using time is simpler because its easier to understand time as a user than block height since blocks have different times.

Problem Definition

No response

Proposed Feature

Modify timeout_height to be time based instead of block height.

We would need to use the blocks time to check the timeout instead of local time. This would simplify the UX for end users since its easier to understand time over heights.

hieuvubk commented 4 weeks ago

So user will pass timeout_height as a timestamp right?

tac0turtle commented 4 weeks ago

we should think about possibly renaming it to timeout. need to have a discussion internally about this

hieuvubk commented 4 weeks ago

Yep, will be a big refactor

alpe commented 3 weeks ago

👍 much better UX from a client perspective

kocubinski commented 3 weeks ago

Tangibly at the API layer, adding a field like

  google.protobuf.Timestamp timeout_timestamp = 5;

would be non-breaking, and timeout_height (at pos 3) would remain in the spec. So I think it looks like we support both? I tried to estimate the complexity of this but couldn't find where TimeoutHeight is used in the SDK.

tac0turtle commented 3 weeks ago

Tangibly at the API layer, adding a field like

  google.protobuf.Timestamp timeout_timestamp = 5;

would be non-breaking, and timeout_height (at pos 3) would remain in the spec. So I think it looks like we support both? I tried to estimate the complexity of this but couldn't find where TimeoutHeight is used in the SDK.

we use it here only: https://github.com/cosmos/cosmos-sdk/blob/720c1086cb3f1c938a8dbd64bf4349154fdbc5eb/x/auth/ante/basic.go#L250-L270, we would make this work with time as well.