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

Time & block height manipulation in tests #1165

Open
leohhhn opened this issue Sep 22, 2023 · 6 comments
Open

Time & block height manipulation in tests #1165

leohhhn opened this issue Sep 22, 2023 · 6 comments
Labels
❓ question Questions about Gno

Comments

@leohhhn
Copy link
Contributor

leohhhn commented Sep 22, 2023

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:

  • A realm contains function that works only before a certain block timestamp / height (i.e. a deadline for voting)
  • Unit tests are needed for the function passing before the specific block timestamp / height, and failing after the block timestamp / height

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:

  • it is not obvious how it is denominated,
  • in order to add 1 to it, you need to .Add(1000000000),
  • it is not clear from the get-go what value it actually represents - is it a timestamp from some specific block, is it a local time from the machine, or is it simply just hardcoded somewhere?

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
Copy link
Member

moul commented Sep 22, 2023

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: #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
Copy link
Contributor Author

leohhhn commented Oct 4, 2023

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

@thehowl
Copy link
Member

thehowl commented Oct 5, 2023

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
Copy link
Member

moul commented Nov 10, 2023

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
Copy link
Member

thehowl commented Nov 13, 2023

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
Copy link
Member

moul commented Nov 15, 2023

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
❓ question Questions about Gno
Projects
Status: 🚀 Needed for Launch
Development

No branches or pull requests

5 participants