Total issuance discrepancy

orriin commented 1 week ago

balance.totalIssuance: 1,195,894,993,084,044 (which should be effectively the number of coins in circulation) subtensorModule.totalissuance: 7,010,451,064,912,511 subtensorModule.totalstake: 5,780,329,269,388,411

mogmachine commented 1 week ago

I think this stems from the way burned and recycled tokens are or are not counted in the various totals, but this is critical we get right as exchanges are relying on us for the truth and we are showing discrepancies in our data.

orriin commented 1 week ago

Initial analysis

Good news:

Balances TotalIssuance amount is consistent with all free + reserved tokens in existence
Subtensor TotalStaked amount is consistent with sum of staked amounts across all accounts
TAOStats reports the correct TI (6.97M)

Inconsistency:

Subtensor TotalIssuance is 35651447307410 (approximately 35k TAO or 0.5%) higher than it should be.

Next Steps

Identify which code path/s decrease the stake or account balance without making a corresponding change to Subtensor TI. Will collab with @mogmachine on that, it sounds like he already has an idea.
Fix the issue, so TI changes no longer deviate from stake or acc balance changes
Adjust the TI back to its correct value
Add a try-state invariant to immediately notify us if state becomes inconsistent again

Long Term

We should migrate our staking system to use Holds instead of the current Stake storage, this can be achieved as a lazy migration. Once migrated to Holds, we can remove all the "double accounting" that we currently do to track total issuance.

orriin commented 6 days ago

Confirmed the cause is with subnet registration fees and tx fees.

Subnet registration makes up for the overwhelming majority of the 35k discrepancy, only ~7.5 TAO comes from tx fees.

Causes

Subnet registration

When a user registers a new network here:

https://github.com/opentensor/subtensor/blob/c097ff8fb0d434a5541b62ab71a9c406e0daa05f/pallets/subtensor/src/lib.rs#L2016-L2023

balance is "burnt" from the registrar without a corresponding update to the subtensor total issuance:

https://github.com/opentensor/subtensor/blob/c097ff8fb0d434a5541b62ab71a9c406e0daa05f/pallets/subtensor/src/root.rs#L833

Tx fees

Tx fees are burnt using the default pallet_transaction CurrencyAdapter:

https://github.com/opentensor/subtensor/blob/c097ff8fb0d434a5541b62ab71a9c406e0daa05f/runtime/src/lib.rs#L339

This default behavior is to just drop (burn) the fee amount, and this updates the Balances pallet TI but not the subtensor pallet TI.

Immediate remedies

Subnet registration

If we wish to consider TAO locked as part of TI, then we should create a TotalSubnetRegistrationLock or similar storage (like we have for TotalStake) and document the subtensor TI storage to explicitly state it is made up of account balances + total stake + total registration lock.

Tx fee

We should write an OnUnbalanced handler for the CurrencyAdapter that will decrease the Subtensor TI when tx fees are paid.

"Syncing" the correct total issuance

Once the above is in place, we can use sudo to initialise the TotalSubnetRegistrationLock value.

Then, we can call a simple admin function that sets the subtensor pallet TI to Balances pallet TI + total stake + total subnet registration lock. This will result in the subtensor pallet TI becoming perfectly balanced.

try-state

A try-state invariant should be written to notify us if TI ever deviates again.

Long term

As mentioned, we should migrate to using Holds for Staking and Subnet Registration which will massively simplify our accounting. However this is a large size lift, and I am not sure how high priority it will be once we fix the current issue.

surcyf123 commented 6 days ago

but tao locked is part of total issuance, so it seems like only the trx fees need to be fixed. And Balances TI should include locked tao

orriin commented 5 days ago

but tao locked is part of total issuance, so it seems like only the trx fees need to be fixed

I think it's still prudent to add a TotalSubnetLocked storage entry, similar to how we have TotalStake.

And Balances TI should include locked tao

This will happen when we migrate to Holds. In the mean time, I think it's prudent to avoid changing the idiomatic definition, or otherwise messing with the Balances pallet TI.

opentensor / subtensor