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

Change block time to millisecond precision in Env #244

Closed
webmaster128 opened this issue Aug 5, 2020 · 9 comments
Closed

Change block time to millisecond precision in Env #244

webmaster128 opened this issue Aug 5, 2020 · 9 comments
Labels
duplicate This issue or pull request already exists on hold Let's pause this and come back in a week...
Milestone

Comments

@webmaster128
Copy link
Member

webmaster128 commented Aug 5, 2020

We get millisecond precision BFT time from Tendermint (if documentation is correct), which is hopefully propagated without precision loss through Cosmos SDK. To the contract we currently expose second precision, which is a somewhat random choice. It has the drawback that it is very well possible to have two blocks at the same time (in seconds).

I suggest exposing millisecond precision to the contract, that is explicitely truncated from the nanosecond time.Time we have:

	blockTime := time.Date(2009, 11, 17, 20, 34, 58, 651787237, time.UTC)
	fmt.Println(blockTime)
	// 2009-11-17 20:34:58.651787237 +0000 UTC
	
	nsec := blockTime.UnixNano()
	fmt.Println(nsec)
	// 1258490098651787237
	
	msec := blockTime.UnixNano() / 1_000_000
	fmt.Println(msec)
	// 1258490098651

https://play.golang.org/p/KAD-F2mc5aX

Unix time in milliseconds is a common time format, e.g. used as the only native time in JavaScript.

I think it would be nice to include that in some kind of breaking change to make it hard for contract developers to create bugs by this change.

ping @ethanfrey

@webmaster128 webmaster128 changed the title Change block time to millisecond precision Change block time to millisecond precision in Env Aug 5, 2020
@ethanfrey
Copy link
Member

I agree with the argument. We can add that to 0.11 / 1.0

I would change the field name from time to time_ms to make sure no contract compiles without change.

@alpe alpe added this to the v1.1.0 milestone Aug 19, 2020
@ethanfrey ethanfrey modified the milestones: v1.1.0, v1.0.0 Aug 19, 2020
@ethanfrey
Copy link
Member

This is blocked on changes to cosmwasm and go-cosmwasm and then coordinate an upgrade

@ethanfrey ethanfrey added the on hold Let's pause this and come back in a week... label Aug 19, 2020
@webmaster128
Copy link
Member Author

@marbar3778 Can you tell us if the BFT time documentation Tendermint is up-to-date with respect to precision: is it still milliseconds? I remember working with higher precision in the Tendermint RPC client.

@tac0turtle
Copy link
Contributor

I don't recall any changes to this. If we did make any changes it would reflect in the docs prior to the release

@webmaster128
Copy link
Member Author

webmaster128 commented Aug 20, 2020

At least in Tendermint 0.32 and 0.33 the block headers have 9 fractional digits in their time.

Cosmos Hub

curl -sS https://cosmos.chorus.one:26657/blockchain | jq '.result.block_metas | map(.header.time)' 
[
  "2020-08-20T11:35:23.471609693Z",
  "2020-08-20T11:35:16.266662354Z",
  "2020-08-20T11:35:08.951653574Z",
  "2020-08-20T11:35:01.791952534Z",
  "2020-08-20T11:34:54.636790218Z",
  "2020-08-20T11:34:47.41957746Z",
  "2020-08-20T11:34:39.879365334Z",
  "2020-08-20T11:34:32.35026125Z",
  "2020-08-20T11:34:24.61344301Z",
  "2020-08-20T11:34:16.591551742Z",
  "2020-08-20T11:34:09.355248484Z",
  "2020-08-20T11:34:02.233298147Z",
  "2020-08-20T11:33:54.834828581Z",
  "2020-08-20T11:33:47.415489489Z",
  "2020-08-20T11:33:40.236825652Z",
  "2020-08-20T11:33:32.858996129Z",
  "2020-08-20T11:33:25.765252737Z",
  "2020-08-20T11:33:18.519162021Z",
  "2020-08-20T11:33:11.448659336Z",
  "2020-08-20T11:33:04.263935638Z"
]

Coralnet

$ curl -sS https://rpc.coralnet.cosmwasm.com/blockchain | jq '.result.block_metas | map(.header.time)'
[
  "2020-08-20T11:31:56.372415067Z",
  "2020-08-20T11:31:50.8515203Z",
  "2020-08-20T11:31:45.015624183Z",
  "2020-08-20T11:31:39.472946648Z",
  "2020-08-20T11:31:33.693216984Z",
  "2020-08-20T11:31:27.884137709Z",
  "2020-08-20T11:31:21.935583233Z",
  "2020-08-20T11:31:16.23334191Z",
  "2020-08-20T11:31:10.606736908Z",
  "2020-08-20T11:31:05.034630692Z",
  "2020-08-20T11:30:59.313595602Z",
  "2020-08-20T11:30:53.792048036Z",
  "2020-08-20T11:30:48.001652605Z",
  "2020-08-20T11:30:42.123857213Z",
  "2020-08-20T11:30:36.277974462Z",
  "2020-08-20T11:30:30.523128934Z",
  "2020-08-20T11:30:24.879305533Z",
  "2020-08-20T11:30:19.329455713Z",
  "2020-08-20T11:30:13.529127952Z",
  "2020-08-20T11:30:07.958099714Z"
]

So if this is the BFT time (which I am not sure about), then the docs are outdated.

Assuming Tendermint provides nanosecond BTF times, I think a better API for CosmWasm would be to keep the time in seconds simple and provide the nanos separately:

pub struct BlockInfo {
    pub height: u64,
    /// Absolute time of the block creation in seconds since the UNIX epoch (00:00:00 on 1970-01-01 UTC).
    ///
    /// The source of this is the [BFT Time in Tendermint](https://docs.tendermint.com/master/spec/consensus/bft-time.html),
    /// converted from nanoseconds to second precision by truncating the fractioal part.
    pub time: u64,
    /// The fractional part of the block time in nanoseconds since `time` (0 to 999999999).
    /// Add this to `time` if you need a high precision block time.
    ///
    /// Using chrono:
    ///
    /// ```
    /// use chrono::{NaiveDateTime, NaiveDate};
    /// let dt = NaiveDateTime::from_timestamp(env.block.time, env.block.time_nanos);
    /// ```
    ///
    /// Creating a simple millisecond-precision timestamp (as used in JavaScript):
    ///
    /// ```
    /// let millis = (env.block.time * 1_000) + (env.block.time_nanos / 1_000_000)
    /// ```
    pub time_nanos: u64,
    pub chain_id: String,
}

@ethanfrey
Copy link
Member

This is also more backwards compatible. Contracts compiled with the old format would run in a newer chain

@webmaster128
Copy link
Member Author

Here is how this change can look for contract developers: CosmWasm/cosmwasm#506

@webmaster128
Copy link
Member Author

VM implementation is done. Blocked by CosmWasm/wasmvm#137. Should be done as part of the 0.11 release.

@ethanfrey
Copy link
Member

Replaced by #271

@alpe alpe added the duplicate This issue or pull request already exists label Dec 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
duplicate This issue or pull request already exists on hold Let's pause this and come back in a week...
Projects
None yet
Development

No branches or pull requests

4 participants