dex: refactor internal indexing into ext traits

[erwanor]

Describe your changes

This PR:

• break out indexing methods from the Inner position manager trait into submodules with crate visibility
• refactor update_position to delegate indexing checks to update_*_index methods
• add a redundant guard against invalid transitions in PositionManager::update_position
• streamline the base liquidity index, fixing a bug with double counting closed positions

This PR contain changes to critical parts of the DEX engine internals.

Checklist before requesting a review

• If this code contains consensus-breaking changes, I have added the "consensus-breaking" label. Otherwise, I declare my belief that there are not consensus-breaking changes, for the following reason:

  This is technically consensus breaking because we fix a bug in the base liquidity index logic, which could trickle down into different DEX execution in some rare cases.

[erwanor]

This is the general idea, probably needs workshopping but i think it's directionally what we want: a safe public DEX api with a small footprint, a private inner implementation that mediate state updates (with a second line of defense against invalid transitions), and a trust boundary behind which we have a variety of internal indices that only filter for relevant state transitions:

///                                                                         
///    ┌─────────────────────────────────────────┐   public DEX component api    
///    │             PositionManager             │   that are wrappers around   
///    │ public interface to the DEX component   │   `update_position` and manage
///    │                   │                     │   the high-level side effects
///    └───────────────────┼─────────────────────┘   like ProtoEvent records   
///                        │                                                
///                        ▼                                                
///  ┌─────────────────────────────────────────────┐                        
///  │   private Inner impl mediate state update   │  update_position with  
///  │                     │                       │    redundant checks    
///  └─────────────────────┼───────────────────────┘                        
///  ■━━━━━━━━━━━━━━━━━━━━━╋━━━━━━━━━━━━━━━━━━━━━━━━━━■    trust          
///                        ▼                              boundary          
///     ┌────────────────────────────────────┐                              
///     │pub(super) trait Index {            │                              
///     │      fn update_index(prev, new, id)│    update_index filters             
///     │                  │                 │    for relevant state transitions      
///     └──────────────────┼─────────────────┘    and deindex/index accordingly           
///                        ▼                                 
///               ┌────────────────┐                         
///               │ internal Inner │  implementation details of the              
///               └────────────────┘  indexing/deindexing logic         

[hdevalence]

@erwanor @zbuc maybe this comment could be clarified / expanded in light of today's discussion in the dex channel on discord, explaining that although the index may seem "backwards", we think it's a heuristic that's both useful and also hard to forge?

[erwanor]

I think we could do this in #4193

[erwanor]

I think the entire index description could be workshopped, we should put some examples

[hdevalence]

Not sure this logic is correct. We do want to update the index when a new position is added, no? Can we refactor this into a match statement like

match (prev_state.map(|prev| prev.state), new_state.state) {
    ...
}

or (if it's cleaner) eliminate that match statement entirely in favor of something where we compute two values:

• a previous contribution to the total, to be subtracted (this is 0 if the position is newly opened)
• a current contribution to the total, to be added (this is 0 if the position is newly closed)

if both are 0 it can short circuit without state changes

[hdevalence]

Oh, hmm, I guess this is sort of what it's doing before with the prev_a, prev_b. I think it would be easier to understand if there wasn't a match statement here and the logic was "bail out if both prev and new amounts are 0".

[erwanor]

I reworked the structure based on your feedback, now we compute contributions upfront:

• compute (prev_a, prev_b) and clamp each value to zero if the position is closed/withdrawn
• compute (new_a, new_b) and clamp each value to zero if the position is closed/withdrawn
• short-circuit if all values are zero (e.g. Closed -> Closed or other "inactive" transitions)

[hdevalence]

Can we add docs to this index describing where it's used? This is the index used for the position eviction logic, but there's another index of assets by inventory that's used for routing heuristics, and we should disambiguate between those.

[erwanor]

Agreed, though I'd prefer to document this with the eviction queue PR