Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Fix] Avoid remove vote when the account transfer all the neo #3720

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 7 additions & 4 deletions src/Neo/SmartContract/Native/FungibleToken.cs
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ protected FungibleToken() : base()

protected override void OnManifestCompose(IsHardforkEnabledDelegate hfChecker, uint blockHeight, ContractManifest manifest)
{
manifest.SupportedStandards = new[] { "NEP-17" };
manifest.SupportedStandards = ["NEP-17"];
}

internal async ContractTask Mint(ApplicationEngine engine, UInt160 account, BigInteger amount, bool callOnPayment)
Expand Down Expand Up @@ -158,10 +158,11 @@ private protected async ContractTask<bool> Transfer(ApplicationEngine engine, UI
else
{
OnBalanceChanging(engine, from, state_from, -amount);
if (state_from.Balance == amount)

state_from.Balance -= amount;
if (StateIsClean(engine, state_from))
engine.SnapshotCache.Delete(key_from);
else
state_from.Balance -= amount;

StorageKey key_to = CreateStorageKey(Prefix_Account).Add(to);
StorageItem storage_to = engine.SnapshotCache.GetAndChange(key_to, () => new StorageItem(new TState()));
TState state_to = storage_to.GetInteroperable<TState>();
Expand All @@ -173,6 +174,8 @@ private protected async ContractTask<bool> Transfer(ApplicationEngine engine, UI
return true;
}

protected virtual bool StateIsClean(ApplicationEngine engine, TState state) => state.Balance == 0;

internal virtual void OnBalanceChanging(ApplicationEngine engine, UInt160 account, TState state, BigInteger amount)
{
}
Expand Down
10 changes: 9 additions & 1 deletion src/Neo/SmartContract/Native/NeoToken.cs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@

using Neo.Cryptography.ECC;
using Neo.Extensions;
using Neo.IO;
using Neo.Persistence;
using Neo.SmartContract.Iterators;
using Neo.SmartContract.Manifest;
Expand Down Expand Up @@ -83,6 +82,15 @@ public override BigInteger TotalSupply(IReadOnlyStoreView snapshot)
return TotalAmount;
}

protected override bool StateIsClean(ApplicationEngine engine, NeoAccountState state)
{
if (engine.IsHardforkEnabled(Hardfork.HF_Echidna))
{
if (state.VoteTo != null) return false;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (state.VoteTo != null) return false;
if (state.VoteTo != null)
{
throw new UncatcheableException("You need to vote for null if you want to transfer all your neos");
}

something like this

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Neat, but then people will complain that "Neo forces us to make two transactions instead of one".

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now can complain about "Neo force me to vote multiple times" and this cost is higher

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Which means people will complain no matter which choice we make. So we can just stick to the good old (proven!) complaint style we have now with "going to zero destroys voting data".

}
return base.StateIsClean(engine, state);
}

internal override void OnBalanceChanging(ApplicationEngine engine, UInt160 account, NeoAccountState state, BigInteger amount)
{
GasDistribution distribution = DistributeGas(engine, account, state);
Expand Down