InitialPoolWeights and TargetPoolWeights should not be []PoolAsset

[sunnya97]

The types for InitialPoolWeights and for TargetPoolWeights in the SmoothWeightChangeParams are both []PoolAsset

type SmoothWeightChangeParams struct {
	StartTime time.Time
	Duration time.Duration
	InitialPoolWeights []PoolAsset
	TargetPoolWeights []PoolAsset
}

But PoolAsset is defined as

type PoolAsset struct {
	Token sdk.Coin
	Weight sdk.Int 
}

But this doesn’t make sense because sdk.Coin includes an amount, but all we want for InitialPoolWeights and TargetPoolWeights is a denom and a weight

I think this comes down to the PoolAsset abstraction just doesn’t make sense

Why are we trying to combine the balance of a pool into the same struct as the weights of the assets? Just to save on storing the denom twice?

A pool’s balances should just be sdk.Coins, and the pool’s weights could be also sdk.Coins, but might make sense to just alias that to a new type called poolWeights (for type safety)

[ValarDragon]

Agreed, I had similar thoughts on the PoolAsset abstraction when going through the proto files earlier. I think this is a refactor we should do, after we hit feature completeness / potentially #post-launch.

[mconcat]

Another thing is that the weight is more like the parameter side data, whereas the assets are treasury side of data. If we want to separate asset management from the pool logic(so the new pool models could just provide curve formulas only), we should separate out weights from the PoolAsset.

[sunnya97]

Yeah, that makes sense

[ValarDragon]

At this point, I'm not sure this is worth the state migration to fix.

At least not in the near future. Going to close for now, and we can re-open if we feel differently later!

[sunnya97]

Is there a way to mark closed issues as "revisit in the future"? Cause if we just close them, I feel we will lose track of them forever.