gnolang / gno

Gno: An interpreted, stack-based Go virtual machine to build succinct and composable apps + gno.land: a blockchain for timeless code and fair open-source.
https://gno.land/
Other
896 stars 375 forks source link

Time & block height manipulation in tests #1165

Open leohhhn opened 1 year ago

leohhhn commented 1 year ago

I am looking to understand how the blockchain context works and can be manipulated within local (unit) tests.

This issue arose when I was looking for a way to set specific values to certain blockchain context variables, such as block timestamp, block height, etc. within tests for realms. In my case, I was stuck trying to set the block timestamp to a specific value.

Here is the specific scenario:

The simplified example realm function is defined as follows:

func vote(deadline time.Time) string {
    if deadline.Unix() <= time.Now().Unix() {
        return "voting already closed"
    }
    votes += 1
    return "voted"
}

I wish to have a test with this flow:

func TestDeadline(t *testing.T) {
    deadline := time.Now().Sub(15 * time.Minute) // or equivalent in gno context

    if vote(deadline) != "voting already closed" {
        t.Fatal("voting after deadline passes")
    }

    deadline := time.Now().Add(15 * time.Minute)

    if vote(deadline) != "voted" {
        t.Fatal("voting before deadline fails")
    }
}

Currently, running

deadline := time.Now()
t.Logf("value of deadline is %d", deadline.Unix())

gives 1234567890, which seems to be a hardcoded value. Although this value is editable (i.e. you can use time.Add, time.Sub, etc), it is quite inconvenient to work with it:

A similar case can be found with std.GetHeight() - which returns 123 upon being called within tests. I have no clue if this value can be manipulated, as it seems that the testing package is not fully complete, as per this comment.

Coming from the Ethereum ecosystem, a local node is required to be running in the background in order to run Solidity tests, due to the nature of the EVM. Hardhat, an Ethereum development framework, opens an API to manage the blockchain context within its local node, and provides helpers to do it.

How do Gno & the GnoVM work regarding this issue? This will most likely be a common dApp developer question, which is why it makes sense to address it.

moul commented 1 year ago

You might want to try using the helper std.TestSkipHeight() (source code) to manipulate the block height in your tests.

We're also working on a similar helper for the block timestamp, which you can find in this pull request: https://github.com/gnolang/gno/pull/569. If reviewed and approved, this could be merged relatively quickly since it only affects the testing helpers. In the meantime, you might be able to implement your locking logic using height comparison.

The fixed values you're seeing are used to ensure deterministic unit tests.

We do plan to provide a system similar to Ethereum's local blockchain for testing, but for now, the current system is the only one supported and it's designed to be lighter (I prefer it over the Ethereum system). However, we do see the value in adding a test suite that can interact with blockchains, so feel free to open an issue to define the need and propose a usage for this.

Regarding your issue with adding 1 vs 10^X, it sounds like you might be adding nanoseconds or a similar unit. Try using 1 * time.Second instead of 1, maybe.

Edit: By the way, I encourage you to use time. helpers instead of converting to unix and making comparisons. I believe your concern with the multiplier comes from this, where the result is not like in bash or php, but includes nanoseconds too. If you use helpers such as time.Before(), time.Add(x * time.Second), I think you won't have such issues and your code will be more concise too.

leohhhn commented 1 year ago

This is the source code where the context is hardcoded. Adding this for future reference.

thehowl commented 1 year ago

I'll close #924 to try to centralise conversation on time/block manipulation in this issue.

This might be a discussion for the next dev call; I think there's two questions which are still unanswered and are subjective IMO, before we merge #569:

  1. Should TestSkipHeights manipulate the timestamp in stdlibs.ExecContext? If so => because gno test and gno.land are decoupled, how should it manipulate the timestamp? Add 5s per each block skipped? Add 1s per each block? Add 1 year per each block?
  2. Should we add a separate TestSetTimestamp that sets the timestamp independently of TestSkipHeights?

In either case, I think (1) we should forego internal/os_test.Sleep, and (2) change TestSkipHeights to only accept positive values (to mark that on the blockchain blocks only move forward!)

IMO, after some more experience working and testing on Gno code, I think it makes sense to have TestSkipHeights implicitly move the time forward, and not implement TestSetTimestamp. And I think that differently from #924, 1 block should be = 1 sec, independently of what we eventually have on the chain, simply because it is easier to write tests without trying to do math on top of your head ("if I want to simulate a call happening 3 minutes later, that's 180 seconds and comes out to be 36 blocks...")

moul commented 1 year ago

I recommend using either two independent helpers or a single helper with two arguments. However, I advise against trying to calculate the timestamp as SkipHeight * 1s.

Additionally, I suggest modifying time.Now() to return block.metadata.timestamp + opcode_counter / precision. This modification would ensure a consistently increasing timestamp within a single transaction.

thehowl commented 1 year ago

Additionally, I suggest modifying time.Now() to return block.metadata.timestamp + opcode_counter / precision.

In test contexts, or also in normal execution?

If you suggest also in normal execution, then I think it's not a good idea. The opcode counter should not be exposed to the execution, otherwise people will depend on it and as such we will have to guarantee the same opcode count consistently.

While we obviously have to do it for determining gas fees, etc., I think this should still be information the code within the VM is not aware of at execution, so we can potentially change it at one point (with a good migration strategy) without fearing breaking existing code in any way.

moul commented 1 year ago

I agree. It's easy to create a deterministic solution that becomes a long-term nightmare to be kept deterministic.

~Perhaps we can pursue a more consistent approach by increasing time each time we create a new frame.~ ~block.metadata.timestamp + frame * 1ms.~

Edit: After discussing it during the public development call, we all agree that it is reasonable for time.now to remain static during transaction execution.