Staking workflow e2e test

[noisekit]

1. Create new random wallet
2. Set ETH balance to 100
3. Set SNX balance to 500
4. Approve SNX spending
5. Create user account
6. Deposit 400 SNX into the system
7. Delegate 300 SNX into the Spartan Council pool
8. Undelegate 200 SNX from the Spartan Council pool
9. Not be able to withdraw because of accountTimeoutWithdraw config
10. Withdraw 200 SNX (after setting accountTimeoutWithdraw to 0)

Some extra notes on implementation and further work

1. At the moment in CI we run all the test on the same Anvil. When new test is added will need to reset Anvil state for each test (evm_snapshot / evm_revert).
2. We are testing against deployed version. Version is taken from toml file. I have seen we do PR with version update (that version may not yet be deployed to the relevant network). May need some input on how to accommodate for the publishing workflow (if it is not going to be somehow fixed by point 3)
3. Next step for the e2e testing is to upgrade contracts on anvil fork and run e2e tests for that. This will resolve problem from point 2 and also give us understanding if tests are going to fail AFTER deploying.
4. Running against currently deployed version will happening by schedule on master branch instead of PRs. For PRs we will release on fork an test on fork with latest deployment configs (point 3)

[dbeal-eth]

Can we move these tests to a separate repo? They can be submodules instead

[noisekit]

Can we move these tests to a separate repo? They can be submodules instead

I intentionally want them to be with the deployment scripts so when we update deployments we do not forget to add tests. They should be collocated with the code that they test.

[dbeal-eth]

adding tests to this repo is likely to cause problems for cannon deployer web ui due to the weight of including unnecessary files in the checkout. If you want to ensure that people are thinking abou tthe tests/adding new tests as necessary as you say, then please add a git hook which verifies that the git submodule is checked out, and if it isnt, block the commit with a error saying to sync submodules. also, you could probably use that opporitunity to remind the user to add tests assuming things are all good

(more non-blocking feedback to follow in isolated comment)

[dbeal-eth]

i am not in support of the inclusion of these tests in our workflow for the following reasons:

• the tests are all redundant. we have very similar tests (and more!) already in our v3 repo. therefore, wouldn't it be better to update the v3 repo to run fork tests along with our current non-fork tests instead? dont believe a test is redundant? please ask and I can provide a link to the corresponding test in the v3 repo
• the original motivation for this PR, to my knowledge, was to verify that configuration changes do not inadvertently lead to dangerous states of the system. for example, shouldn't the first test in this suite be the one raised by @kaleb-keny for example, checking that the c-ratio is above 1.0? or vice versa, checking that a c-ratio change does not trigger every single account in the system to be immediately liquidated
• this adds about 1000 lines of additional code that we have to remember/think to maintain on top of all the other stuff it seems like we are barely keeping up with. even if we included it in the repo mandatory and maybe added it to the runsheet, it seems like extremely optimistic thinking to believe that we are actively going to go out and update tests every time before a release
• ive said this before but e2e tests are notoriously the last reliable and most difficult to maintain tests. coupled with the fact that these tests are extremely redundant, its likely these tests will be a barrier which has a high noise to signal ratio, making the tests a nuisance we like to ignore instead of a source of help.

as a solution, i think we should consider implementing something more like shadow forking. basically, every time a user intracts with our system, we attempt to run the same transaction on a fork. if the transation fails, that gets added to a report. This gives an excellent picture of what a chance is likely to have in terms of impact on actual transactions on mainnet. This type of system would definitely catch the case of liquidation due to incorrect c-ratio.

[noisekit]

• These tests are not redundant. They are e2e tests over existing deployment (and potential deployment) that we do not have in v3.
• They are testing actual chain deploy and not just the protocol code.
• They are also very reliable. We had almost zero flakiness over past months in v3ui
• Adding extra check to ensure deployment is not breaking things BEFORE changes are deployed is quite crucial. Checking ui after deploy would be too late as everything can be already bricked.
• The tests code is intentionally as dumb as possible with zero abstraction beyond single operation utils, so supporting them should not be an issue
• cannon should not checkout git, diff needs to be embedded into the state. This solves the problem with repo size. But even with current size cannon can checkout only tomls subfolder + chain config. That should eliminate size issue too

I believe this counters all the points @dbeal-eth and I do believe this PR should be merged.

[dbeal-eth]

• These tests are not redundant. They are e2e tests over existing deployment (and potential deployment) that we do not have in v3.

Ok, but the actual test that is being performed, regardless of the environment it is performed, is the same. Why don't we instead update our existing tests to work for this on the v3 repo?

• They are testing actual chain deploy and not just the protocol code.

Previous comment. The actual tests are the same as the ones performed on the protocol code deploy, just not on live chain/fork.

• They are also very reliable. We had almost zero flakiness over past months in v3ui

I'm also concerned about tests failing because the spec changed (as intended)

• Adding extra check to ensure deployment is not breaking things BEFORE changes are deployed is quite crucial. Checking ui after deploy would be too late as everything can be already bricked.

Yes I agree. So we do shadow deployment before the deployment as you say and we gain the insight.

• The tests code is intentionally as dumb as possible with zero abstraction beyond single operation utils, so supporting them should not be an issue

Every line of code that we have to maintain is a burden

• cannon should not checkout git, diff needs to be embedded into the state. This solves the problem with repo size. But even with current size cannon can checkout only tomls subfolder + chain config. That should eliminate size issue too

Not possible because of the way git protocol works. We can't work by diff no matter what we do

I believe this counters all the points @dbeal-eth and I do believe this PR should be merged.

I have countered your counters I believe

[0xjocke]

I would say that these are great smoke tests to have. This covers create account, deposit, delegate, undelegate and withdraw, if that doesn't work something is most likely wrong. I would say mint and borrow should be part of these core tests too.

Then probably we'd want to add some smoke tests for spot and perps too.

[0xjocke]

I guess the tricky thing is when we expect the networks to differ, eg. base currently. Not sure how often we could have changes that requires the core LP interactions to fail.. But it would be a challenge if we add more complicated tests.

[noisekit]

I guess the tricky thing is when we expect the networks to differ, eg. base currently. Not sure how often we could have changes that requires the core LP interactions to fail.. But it would be a challenge if we add more complicated tests.

It will not be a challenge as if networks is different we jsut have a separate tests. Tests are not supposed to be abstract and complicated. Copypaste would be the way.

Don't want to be a blocker but my thoughts on this issue have not changed

[noisekit]

optimism goerli is temporarily disabled until we figure out what is wrong with it.