Testing with Expiring Storage

[wraith shell]

The SDK doesn't contain convenient functionality for testing when storage expires.

Has anyone found any semi-convenient ways to write these types of tests?

@raven notch I think we had discussed this problem recently. Did you end up finding a way to do this?

[restive copper]

With @prime ether we're working on making soroflare work for this (mainly testing client side).

[raven notch]

Storage expiration is something I've dealt with in testing a bunch, but I don't have any reusable code for it. The token fuzzer I'm working on doesn't explicitly try to force state expiration, but it does trigger it by advancing time.

The first step to testing state expiration is advancing time reliably, which is tricky, and the way it's done in the docs (by mutating the Env sequence number / timestamp) is imo not sufficient, particularly because of issues mentioned in https://github.com/stellar/rs-soroban-sdk/issues/1162#issuecomment-1892686135 wrt how the test environment manages storage.

The token fuzzer's code for advancing time is https://github.com/brson/soroban-token-fuzzer/blob/master/src/fuzz.rs#L592, but is not written in a generalized way that the SDK could reuse directly.

I would though suggest that the SDK should provide a correct way to advince time in tests as a first step, and the docs should be updated to use it. I am willing to propose something here if desired.

With that in place it could do things that are more clever like introspecting the storage and advancing time strategically to force various state expirations.

[raven notch]

The most important thing to note is that when advancing time, stale storage entries must be purged; the test env does not do this on its own; and there's a routine in the linked code showing how to do it for snapshots.

If stale storage entries are not purged the runtime will not operate as it does in production, as in prod the runtime does not ever see these stale entries.

[copper matrix]

before jumping into implementation issues, can we first discuss how it would be used ?

I think what matters at the SDK/contract level is not expiration, but that the bumps happen as expected. I'm not sure if advancing time is necessary to verify that

i.e. I'm not sure if it's more productive to test expiration via advancing time vs testing that the ledger entries have the expected expiration

though I guess that could be interesting for the temp entries?

[wraith shell]

Testing is also about knowledge sharing and understanding how the system works, so I think it is important that the test framework allows a developer to advance time so that they can write tests that knowledge share with the same concepts as the real world.

[copper matrix]

Well, contracts never interact with expired entries, so there is not much to cover. Which is why we need to figure out some model test cases we'd want to support

[wraith shell]

contract never interact with expired entries
Use cases do though.

[ancient flume]

Maybe what we could do is something like: give the fuzzer control over which temp entry to expire (a new argument: map of entry->bool), default would be "all entries" can be expired. contract devs then have to annotate their test cases to exclude certain expiration scenario (ie: they're ok with a panic so no need to explore that branch)

in general it may be useful to give some control over ledger state to the fuzzer maybe it's already possible?

[copper matrix]

what I mean is that interaction with expired entries is kind of UB. we can define it of course. here is what I suggest:

• for testing the bumps add functionality to assert on the expiration ledger of the entries
• imitate the network behavior when ledger sequence is bumped (not so much for regular unit testing, rather for scenarios like fuzzing): when accessing a persistent expired entry trap with non-recoverable error. that's not exactly the same behavior as core has, but I don't think we can do much better. when accessing an expired temp entry, treat it as non-existent (might as well actually remove it lazily). implementation-wise this boils down to adding some test-only hooks to the storage implementation

[ancient flume]

What I was thinking about is a different scenario: help contract dev find when they accidentally have a dependency on a temp entry that would cause them to panic when they expire post setup (causing their contract to get wedged)

[copper matrix]

It's fine did temp entries to expire though...

Also, we shouldn't create a wrong impression that entries have to be bumped in contact calls; bumps don't have to be atomic

[ancient flume]

actually for all of those scenarios, we may not need something super complicated. What could be done is something where we provide test helpers/test hooks (not available in the "real" env) as to enumerate interesting scenarios. If I could write a test structured like this:

1. setup (create contract, a bunch of entries)
2. helper_call lambda
   • where "lambda" is the continuation of the test (entire test is really testing a bunch of full end to end scenarios)
   • helper_call backups current state and loops over interesting scenarios
     • for each scenario, restores state, tweaks it (bumps, expires entries, etc) and invoke lambda
       (the chaining of scenario could actually go deeper)

[copper matrix]

I'm not sure what would that achieve honestly

again, the only thing you can control from within the contract is bumping the entries. restoration and expiration are external to the contract. I can get behind the idea of doing something similar to what core would do when entry expires (i.e. crash for persistent/delete for temp), but I'm not sure if deeper support is necessary. we definitely need to expose some simple way to assert on entry expiration after the contract call

[wraith shell]

Yup I think something like this would suffice, at least to begin with then we figure out where the gaps are. As a starting point helper functions to advance time, that does the same or similar things to what @raven notch posted above here: #1202349664196632677 message. I think we could integrate that idea with the SDK, as a function like env.ledger().advance(N).

Advancing state and rewinding should already be possible because that was the motivation for the design of the env as a non-singleton and it's deep cloning behavior, although we haven't tested it in a while and haven't used the test strategy much in the SDK yet.

[raven notch]

One of the problems I have run into repeatedly is that when I hit a "MissingValue" error during testing, I don't get enough information to figure out what was missing or why. As with other errors during testing, in these cases I end up println-instrumenting the runtime to figure out what happened.

Lack of feedback on errors during testing is a recurring problem for me. This is not related to state expiration, but I have a habit of using the try_foo client methods in my tests so that I can handle errors, and have recommended others do this, but this has a problem: the fidelity of the errors that come out of these calls is almost too low to be useful. e.g. if I call the foo method and the call panics I will (i think) see a diagnostics dump that is somewhat useful; but the results from try_foo just contain an ScError code, and that code has in some cases been "sanitized" so that (as I recall) internal runtime errors are mostly not exposed to the user.

[wraith shell]

I hit this problem last week as well.

[raven notch]

I ran into a new problem kind of related to state expiration testing tonight, though it's late and I don't fully understand it. As I've mentioned before I recreate the Env each simulated "transaction" while advancing time; by exporting and importing a snapshot; and while I do the the snapshot, I also purge expired storage, which is necessary to avoid panics in the runtime.

I recently discovered though that every fresh Env has a prng seed of 0, and this can cause problem with mocked recording auths (a subject I don't know anything about), where identical nonces are generated across multiple Env's, triggering an error.

I hadn't seen this previously presumably because I was manually generating the needed auths, with unique nonces, but for some recent code I was writing the correct auths are complicated so I was just calling mock_all_auths, and the call ended up generating multiple identical nonces on the AccountAuthorizationTracker::new_recording code path, due to the reused 0 base prng seed.

I'm presently working around this with a patch to the SDK that allows setting the base prng, but I need to sleep on it now and understand it better.

[wraith shell]

Ah. I can see how that would be a problem.

The base prng was set to 0 to make tests reproducible, largely because we take snapshots on every test run to quckly be able to detect divergence in behavior with sdk upgrades and protocol changes.

We could try and some up with a smart scheme for the env base prng, where it's 1, then 2, etc. Still predictable.

Anotehr option is using the env snapshot instead of a ledger snapshot. I don't love that we have both, but an env snapshot will capture more of the Env's test state. You can get to it by using env.to_snapshot() instead of env.to_ledger_snapshot(). The env snapshot doesn't hold a copy of the base prng right now, but it was be reasonable to add it.

Separately I think we need to address the issue of testing time advancement and expiring storage. If we did that this presumedly wouldn't be an issue external to the env.