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

Remove empty keys #825

Merged
merged 8 commits into from
Jun 15, 2019
Merged
Changes from 3 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
24 changes: 19 additions & 5 deletions neo/SmartContract/InteropService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -546,16 +546,30 @@ private static bool PutEx(ApplicationEngine engine, StorageContext context, byte
if (value.Length > MaxStorageValueSize) return false;
if (context.IsReadOnly) return false;
if (!CheckStorageContext(engine, context)) return false;

StorageKey skey = new StorageKey
{
ScriptHash = context.ScriptHash,
Key = key
};
StorageItem item = engine.Snapshot.Storages.GetAndChange(skey, () => new StorageItem());
if (item.IsConstant) return false;
item.Value = value;
item.IsConstant = flags.HasFlag(StorageFlags.Constant);
return true;

StorageItem item = engine.Snapshot.Storages.TryGet(skey);
if (item?.IsConstant == true) return false;

if (value.Length == 0)
{
// If is empty, we try to remove it

engine.Snapshot.Storages.Delete(skey);
return true;
Copy link
Member

Choose a reason for hiding this comment

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

If I put an empty bytes with StorageFlags.Constant flag, what will happen?

Copy link
Member Author

Choose a reason for hiding this comment

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

The expected behaviour is: write nothing, read empty bytes

Copy link
Contributor

@igormcoelho igormcoelho Jun 13, 2019

Choose a reason for hiding this comment

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

What's an example use case for const? I think you're right... logically speaking, if someone wants to keep a constant value, that value could be zero. So we need to keep it allow it to be unchanged in the future. We cannot pre-reap const empty values @shargon, because it may be part of contract logic, and perhaps be re-written in the future because we accidentally pre-dropped its "constness".

Copy link
Contributor

@igormcoelho igormcoelho Jun 13, 2019

Choose a reason for hiding this comment

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

Only explicit Delete will destroy them, right @erikzhang ? I just noticed Delete cannot reap const values too. So these are contract-lifetime values, correct? Only Contract.Destroy or Migrate can reap them.

}
else
{
item = engine.Snapshot.Storages.GetAndChange(skey, () => new StorageItem());
erikzhang marked this conversation as resolved.
Show resolved Hide resolved
item.Value = value;
item.IsConstant = flags.HasFlag(StorageFlags.Constant);
return true;
}
}

private static bool Storage_Put(ApplicationEngine engine)
Expand Down