example-tictactoe icon indicating copy to clipboard operation
example-tictactoe copied to clipboard

Occasionally game completed time is far in the past

Open mvines opened this issue 6 years ago • 5 comments

eg, image

No good STR right now beyond waiting for user reports, this could use some investigation.

I suspect that we're getting a timestamp of 0 somehow since we use UNIX epoch in the tictactoe webapp.

mvines avatar Jan 18 '19 15:01 mvines

I'm now seeing this issue consistently after any game

mvines avatar Mar 20 '19 14:03 mvines

@mvines looks like both programs use tick_height now instead of a client-provided timestamp.

https://github.com/solana-labs/example-tictactoe/blob/master/program-native/src/lib.rs#L179 https://github.com/solana-labs/example-tictactoe/blob/master/program-bpf/src/tictactoe/tictactoe.c#L385

This appears to have changed in this commit:

https://github.com/solana-labs/example-tictactoe/commit/ae874e9b3330bc58996bee9d457ed21fd5bf9fe0

The Solana Rust SDK looks like it has a timestamp() function, but I couldn't find one in the BPF sdk:

https://github.com/solana-labs/solana/blob/master/sdk/src/timing.rs

Do you have a preference for how this gets fixed?

  1. very lightweight, no program change: web client keeps track of tick_height to epoch millis
  2. similarly lightweight, no program change: node "server" keeps track of tick_height to epoch millis
  3. least lightweight, program change(s): program state include epoch millis as data structure

Thank you in advance for any advice you may have!

sunnygleason avatar Apr 15 '19 17:04 sunnygleason

(3) would be nice as a longer term fix. In the immediate term I'd be fine with option (0), just hide the recent game time from the UI though :)

mvines avatar Apr 15 '19 17:04 mvines

@mvines awesome, (0) is already implemented on the custom-testnet branch:

https://github.com/solana-labs/example-tictactoe/commit/16052178d6a53ce6d80d12eabf7d0f6153425bed#diff-1e5251f0c8caa1a663183aa6b0bd36e3R221

I'll start a dev task to make the changes in (3) over the next few days (there are probably a couple visualization tasks which are higher priority)

sunnygleason avatar Apr 15 '19 17:04 sunnygleason

Please revert 458adceed5d3163bd555dca8f1cf8f287cf2e384 as a part of fixing this issue

mvines avatar May 02 '19 23:05 mvines