We should upgrade Hardhat and our contracts to >= 0.8.4, this will give us the opportunity to use new features, such as the new panic errors.
Once again, we need to be sure that our testing environment will be compatible with this new version.

Right now, anyone would be able to grief a token pair by making the reverse incorrect pair, for example dai as the risky and eth as the stable asset. This would make a call to deploy an engine with eth as risky and dai as stable revert, because its already defined in the reverse mapping.

Pools are initialized with a time until expiry. Whenever a swap happens, we should be taking the current timestamp and calculating the new time until expiry. This value should be an input to the trading function, not the time until expiry value we initialize the pool with.

Invariant checks can be removed

Related to #97

When time changes, the curve translates. In order to translate, more tokens must be provided to the reserves, both risky and stable. To be able to do this, there must be some balance which can be drawn upon to update time delta.

One possible solution is to pool the fees, and draw from the fee pool when time delta tau is updated.

Need to clean up the engine functions while also avoiding stack too deep errs!

Engine contract needs to store reserve data so we can query it on-chain

Reserve library isn't packing any of its many reserve values

https://github.com/primitivefinance/primitive-v2-core/blob/develop/contracts/libraries/Margin.sol

margin previously had a bytes32 marginId, but this was taken out in favor of a standard address -> Margin.Data mapping. The fetch function should be removed from the lib.

https://github.com/primitivefinance/primitive-v2-core/blob/95108c8f4b370f1bfb01ccf7d779f3b89ad6b907/contracts/libraries/Reserve.sol#L33-L39

Update could be called even if the reserves never change. There should be a check to make sure reserve values have changed since last update.

The allPools array can be implemented off-chain by indexing events for all the new pools created.

In the switch case, if we havn't deployed the factory first, then we can't deploy the other contracts. There is an order we must deploy the contracts. This order is enforced only by the contract name strings in the array, so the position in the array matters to the deployment; but it shouldn't.

https://github.com/primitivefinance/primitive-v2-core/blob/863ee491c67a0126c751e4e1a83658c5375c5c8b/test/unit/primitiveEngine/effect/create.ts#L17

House contracts are not being audited so it should not be in this repo

In the Reserve.sol library there is an internal function _blockTimestamp() to get the uint32 block timestamp. This is called within the library. We should move this and make it an argument in the update function, and have _blockTimestamp() implemented in the contract which is importing the library instead, like the PrimitiveEngine.sol contract

Most the Engine functions can most likely be improved. Whether that's improving gas by storing state vars in memory, adding comments, improving readability, or reducing code.

We don't need to return a storage slot variable if it is a parameter, because the storage slot is referencing the same value

A lot of the math, swap math, liquidity math, etc. can be exported to library contracts.

Engine contract does not have any fee related code. This also means changes in the trading function math, which is gamma (1 - fee) multiplied by the deltaOut.

The Base Engine contract should have virtual functions which are implemented by the Engine contract; the common denominator.

Whats an Engine?

• Its an AMM with some way to manipulate liquidity through a trading function.

The file doesn't have much and the data struct doesn't have optimizations yet--- ahh, this is why I had calibration struct. Its so we can optimize the storage slot

TypeChain 5 comes with a caching feature that will speed up the compilation and testing. We need to upgrade from 4 to 5. However we have to be sure that this is not going to break our testing environment. My previous tests were not successful because of an issue when the deployed contracts were casted in the fixtures.

We give some tokens to an engine and signer with:
erc20.mock.balanceOf(signer).returns(amount)

Then we call the create function on a target contract (which should be a TestCallee). TestCallee will then call engine.create().

In engine create, an amount of risky and stable amount is calculated to be the initializing reserve values. These reserve values are set, and now we need to get those reserve balances as actual tokens in the engine.

The engine stores the risky and stable balances.
The engine then calls the msg.sender (our TestCallee contract) using createCallback()
Then in the callback, TestCallee will pull tokens from the signer, and send them to the engine.
The Engine will then check the current risky and token balance against the balances saved in memory prior to the callback. If tokens were sent in during the callback, there would be a difference.

The problem with the mock contract, is we can't increment the engine balance of tokens during the callback with a mock

Liquidity shares must be normalized so that 1 share = 1 covered call. This means the liquidity shares will scale down the reserves.

They should be directed to all users with margin balances?

https://github.com/primitivefinance/primitive-v2-core/blob/95108c8f4b370f1bfb01ccf7d779f3b89ad6b907/contracts/libraries/Position.sol#L9-L11

Unused callback

https://github.com/primitivefinance/primitive-v2-core/blob/95108c8f4b370f1bfb01ccf7d779f3b89ad6b907/contracts/libraries/Position.sol#L26-L33

entropy of position ids are questionable

Grants more flexibility and risk to callback functionality

• Separate periphery
• Separate sim
• Separate deployment environment