Closed DariusParvin closed 3 years ago
Thanks for your feedback @marsella! I like the idea of storing the customer dbs in channel specific directories so that we can run tests on different channels in parallel.
The only issues is when we restore
the state, I think that would interfere with other channels that are being run in parallel.
More generally though, I don't think zeekoe can reliably broadcast on-chain operations concurrently for multiple channels with the same tezos account (discussed in #212). In short, the customer or merchant may run into issues if one of them tries to perform two on-chain operations (e.g. establish on channel-2 and close on channel-1) at the same time.
With the current setup, we can do tests where the customer has multiple channels open with the merchant (making sure to run on-chain operations sequentially). However, I think having a test framework that can run independent tests in parallel would take some thinking to get correct and might be better as a separate issue/PR? If you agree then I'll create a separate issue for that and leave this test as is.
Ayo, my concern with using a random temp dir is that the current command loop is stateless -- we'd have to add a way to save the temp dir across the lifetime of a loop. I suggested the channel ID because it's deterministic.
Darius, I don't understand your concern with restore
interfering with other parallel channels. As long as each channel has a unique ID, restoring one (from a channel-id-specific directory) should be independent of the stored states of the other.
I had forgotten about the Tezos limitations to posting in parallel, though. Please do make an issue for that (specific to testing). I still have a slight preference to changing the file thing now anyway, but if you don't, just make sure it's noted in the issue.
Ayo, my concern with using a random temp dir is that the current command loop is stateless -- we'd have to add a way to save the temp dir across the lifetime of a loop. I suggested the channel ID because it's deterministic.
Ah, great point. We do want it to be deterministic.
@marsella an example of what I mean is let's say the customer has two channels, A and B. A is to test dispute, and B is to test normal operation. A store
s the db, then restore
s the db at a later time just before close
. If B had made any payments before restore
and the channel is still open, then B will be working with a revoked state, no?
Oh, I see. Then yes, that sounds like something for a future issue. Thanks for clarifying.
Thanks a lot for the thorough review! I learned a lot from the feedback. It's ready for re-review @yaymukund
Rather than adding a command directly for disputing, I added two commands
store
andrestore
for the customer db so we can be more flexible around which revoked state gets broadcast. e.g.We can also test what happens if the customer attempts to make a payment on a revoked state e.g.