[Bug] deal_provider_collateral_bounds implementation does not match that in Lotus.

[ruseinov]

Lotus:

    lockTargetNum := big.Mul(ProviderCollateralSupplyTarget.Numerator, networkCirculatingSupply)
    lockTargetDenom := ProviderCollateralSupplyTarget.Denominator
    powerShareNum := big.NewIntUnsigned(uint64(pieceSize))
    powerShareDenom := big.Max(big.Max(networkRawPower, baselinePower), powerShareNum)

    num := big.Mul(lockTargetNum, powerShareNum)
    denom := big.Mul(lockTargetDenom, powerShareDenom)
    minCollateral := big.Div(num, denom)
    return minCollateral, builtin.TotalFilecoin

rust builtin-actors:

    let lock_target_num = network_circulating_supply * policy.prov_collateral_percent_supply_num;
    let power_share_num = BigInt::from(size.0);
    let power_share_denom = max(max(network_raw_power, baseline_power), &power_share_num).clone();

    let num: BigInt = power_share_num * lock_target_num.atto();
    let denom: BigInt = power_share_denom * policy.prov_collateral_percent_supply_denom;
    (
        TokenAmount::from_atto(num.div_floor(&denom)), // to fix this it has to be div_euclid
        TOTAL_FILECOIN.clone(),
    )

Happy to contribute a fix if the above seems reasonable.

[Stebalien]

These look identical to me. What am I missing?

[ruseinov]

These look identical to me. What am I missing?

big.Div is actually BigInt::div_euclid and not div_floor.

[Stebalien]

Oh, I see. Both the numerator and the denominator should be non-negative, so this shouldn't matter as far as I can tell. We could change the Lotus version to call Quo, but if we have negative numbers here, something is very wrong.

[ruseinov]

Oh, I see. Both the numerator and the denominator should be non-negative, so this shouldn't matter as far as I can tell. We could change the Lotus version to call Quo, but if we have negative numbers here, something is very wrong.

Yeah, I guess you are right. Still, it would be nice to see aligned implementations. I have found this discrepancy while working on Filecoin.StateDealProviderCollateralBounds RPC endpoint and the outputs of Lotus and Forest don't match. This particular issue does not seem to affect the output though. I had to analyse every single line of code related to state_deal_provider_collateral_bounds calculation and having different division strategies is just another thing that needs to be eliminated from the list of potential causes.

[Stebalien]

In that case, feel free to make a PR to change the version in go-state-types to Quo.

(closing because this isn't really a bug)

[ruseinov]

In that case, feel free to make a PR to change the version in go-state-types to Quo.

(closing because this isn't really a bug)

Will do.