Conversation
* remove balancer tests (we will be actively removing balancer support itself by the next release) * update to use latest openzepplin (actually supports 0.8) * remove the foundry warnings error (temp so that we can get things to compile)
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
Warning Review the following alerts detected in dependencies. According to your organization's Security Policy, it is recommended to resolve "Warn" alerts. Learn more about Socket for GitHub.
|
there are probably some dumb things here and there but this serves as a starting point now moving onto coverage, there are some things missing
|
Also, @kaze-cow please remember to set the correct base branch to see the actual changes. You are based in |
anxolin
left a comment
There was a problem hiding this comment.
Should we cleanup and get these merged?
forge build fails for me
--> src/contracts/GPv2Settlement.sol:7:1:
|
7 | import {IGPv2Settlement} from "./interfaces/IGPv2Settlement.sol";
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Can we make sure tests are passing?
| /// protection mixed in so that signed orders are only valid for specific | ||
| /// GPv2 contracts. | ||
| bytes32 public immutable domainSeparator; | ||
| bytes32 public immutable DOMAIN_SEPARATOR; |
There was a problem hiding this comment.
Changes the public getter function name, this affects the ABI, is this intended?
and also should it be in e2e migration PRs or should it be an independent one?

Description
Forking off the original
e2e-migrationbranch, finish the E2E tests migration. This PR has been pending for a while and needed some updates based on the latest state of solidity/plans for the future of the protocol, so its been updated/adapted.Concretely, this branch differs from the original branch by:
forge fmtyarnto fail install on my ARM hardware and updating seems to have no noticable negative effect at this time)The actual diff between original e2e testing brnach can be seen here (individual commits viewing is helpful due to the forge fmt): e2e-migration...e2e-migration-update
The previous branch was pretty scant on documentation and review, and never received a review in the first place. The original reviewers are no longer here, and some lint fixes from upstream foundry are needed, so may we just start again with this branch?
Test Plan
The new tests can be verified with
FORK_URL=<url of mainnet> forge test. There are no more tests in hardhat. We may consider removing hardhat entirely?Related Issues
#226