penumbra-zone / penumbra

Penumbra is a fully private proof-of-stake network and decentralized exchange for the Cosmos ecosystem.
https://penumbra.zone
Apache License 2.0
377 stars 296 forks source link

Enshrine DEX position `check_stateless` and `check_stateful` expectations in tests #2421

Open erwanor opened 1 year ago

erwanor commented 1 year ago

In Penumbra, ActionHandlers define the interface for handling transaction actions, and in particular, assert their validity via check_stateless and check_stateful. With respect to the DEX component, it would be helpful to enshrine the expectations set forth in both functions in unit-tests that would control for any accidental regression.

For example, the dex routing logic builds off a certain number of assumptions such as:

    async fn check_stateless(&self, _context: Arc<Transaction>) -> Result<()> {
        // Check:
        //  + reserves are at most 112 bits wide,
        //  + at least some assets are provisioned.
        self.position.reserves.check_bounds()?;
        // Check:
        //  + the trading function coefficients are at most 112 bits wide.
        //  + the trading function coefficients are non-zero,
        //  + the trading function doesn't specify a cyclic pair,
        //  + the fee is <=50%.
        self.position.check_stateless()?;

It would make the codebase more robust to have tests that make sure that check_stateless, check_stateful always catch some positions that we craft to be invalid. This is a good first issue for prospective contributors, and I would be happy to help you ship this.

hdevalence commented 8 months ago

This is the type of test that would be ideal to write using the infrastructure that will land with https://github.com/penumbra-zone/penumbra/issues/3588#issuecomment-1930965418