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

nullable Storage.Get; refactor TokenContract #1214

Open
wants to merge 21 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all 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
4 changes: 2 additions & 2 deletions src/Neo.SmartContract.Framework/ByteString.Extension.cs
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ public static int IndexOf(this ByteString byteString, ByteString byteToFind)
/// <returns>True if start with</returns>
public static bool StartWith(this ByteString byteString, ByteString byteToFind)
{
return StdLib.MemorySearch(byteString, byteToFind) == 0;
return Helper.NumEqual(StdLib.MemorySearch(byteString, byteToFind), 0);
}

/// <summary>
Expand All @@ -104,7 +104,7 @@ public static bool StartWith(this ByteString byteString, ByteString byteToFind)
/// <returns>True if ends with</returns>
public static bool EndsWith(this ByteString byteString, ByteString byteToFind)
{
return StdLib.MemorySearch(byteString, byteToFind) + byteToFind.Length == byteString.Length;
return Helper.NumEqual(StdLib.MemorySearch(byteString, byteToFind) + byteToFind.Length, byteString.Length);
}

/// <summary>
Expand Down
12 changes: 6 additions & 6 deletions src/Neo.SmartContract.Framework/Linq/LinqExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@
count++;
sum += item;
}
if (count == 0) throw new Exception("source is empty");
if (Helper.NumEqual(count, 0)) throw new Exception("source is empty");
return sum / count;
}

Expand All @@ -153,7 +153,7 @@
count++;
sum += selector(item);
}
if (count == 0) throw new Exception("source is empty");
if (Helper.NumEqual(count, 0)) throw new Exception("source is empty");
return sum / count;
}

Expand All @@ -173,7 +173,7 @@
count++;
sum += item;
}
if (count == 0) throw new Exception("source is empty");
if (Helper.NumEqual(count, 0)) throw new Exception("source is empty");
return sum / count;
}

Expand All @@ -196,7 +196,7 @@
count++;
sum += selector(item);
}
if (count == 0) throw new Exception("source is empty");
if (Helper.NumEqual(count, 0)) throw new Exception("source is empty");
return sum / count;
}

Expand All @@ -216,7 +216,7 @@
count++;
sum += item;
}
if (count == 0) throw new Exception("source is empty");
if (Helper.NumEqual(count, 0)) throw new Exception("source is empty");
return sum / count;
}

Expand All @@ -239,7 +239,7 @@
count++;
sum += selector(item);
}
if (count == 0) throw new Exception("source is empty");
if (Helper.NumEqual(count, 0)) throw new Exception("source is empty");
return sum / count;
}

Expand All @@ -253,7 +253,7 @@
/// <exception cref="ArgumentNullException">source is null.</exception>
public static bool Contains<T>(this IEnumerable<T> source, T value)
{
return Any(source, s => s.Equals(value));

Check warning on line 256 in src/Neo.SmartContract.Framework/Linq/LinqExtensions.cs

View workflow job for this annotation

GitHub Actions / Test

Dereference of a possibly null reference.
}

/// <summary>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
<Project Sdk="Microsoft.NET.Sdk">

<PropertyGroup>
<Nullable>enable</Nullable>
<TargetFramework>net8.0</TargetFramework>
<AssemblyTitle>Neo.SmartContract.Framework</AssemblyTitle>
<AssemblyName>Neo.SmartContract.Framework</AssemblyName>
Expand Down
4 changes: 2 additions & 2 deletions src/Neo.SmartContract.Framework/Nep17Token.cs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ namespace Neo.SmartContract.Framework
[ContractPermission(Permission.Any, Method.OnNEP17Payment)]
public abstract class Nep17Token : TokenContract
{
public delegate void OnTransferDelegate(UInt160 from, UInt160 to, BigInteger amount);
public delegate void OnTransferDelegate(UInt160? from, UInt160? to, BigInteger amount);

[DisplayName("Transfer")]
public static event OnTransferDelegate OnTransfer;
Expand Down Expand Up @@ -64,7 +64,7 @@ protected static void Burn(UInt160 account, BigInteger amount)
PostTransfer(account, null, amount, null);
}

protected static void PostTransfer(UInt160 from, UInt160 to, BigInteger amount, object data)
protected static void PostTransfer(UInt160? from, UInt160? to, BigInteger amount, object? data)
{
OnTransfer(from, to, amount);
if (to is not null && ContractManagement.GetContract(to) is not null)
Expand Down
2 changes: 1 addition & 1 deletion src/Neo.SmartContract.Framework/Services/Contract.cs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ public class Contract
public readonly ContractManifest Manifest;

[Syscall("System.Contract.Call")]
public static extern object Call(UInt160 scriptHash, string method, CallFlags flags, params object[] args);
public static extern object Call(UInt160 scriptHash, string method, CallFlags flags, params object[]? args);

[Syscall("System.Contract.GetCallFlags")]
public static extern CallFlags GetCallFlags();
Expand Down
8 changes: 4 additions & 4 deletions src/Neo.SmartContract.Framework/Services/Storage.cs
Original file line number Diff line number Diff line change
Expand Up @@ -37,13 +37,13 @@ public static extern StorageContext CurrentReadOnlyContext
/// Returns the value corresponding to the given key for Storage context (faster: generates opcode directly)
/// </summary>
[Syscall("System.Storage.Get")]
public static extern ByteString Get(StorageContext context, ByteString key);
public static extern ByteString? Get(StorageContext context, ByteString key);

/// <summary>
/// Returns the value corresponding to the given key for Storage context (faster: generates opcode directly)
/// </summary>
[Syscall("System.Storage.Get")]
public static extern ByteString Get(StorageContext context, byte[] key);
public static extern ByteString? Get(StorageContext context, byte[] key);

/// <summary>
/// Writes the key/value pair for the given Storage context (faster: generates opcode directly)
Expand Down Expand Up @@ -100,8 +100,8 @@ public static extern StorageContext CurrentReadOnlyContext
public static extern Iterator Find(StorageContext context, byte[] prefix, FindOptions options = FindOptions.None);

#region Interface with default Context
public static ByteString Get(ByteString key) => Get(CurrentReadOnlyContext, key);
public static ByteString Get(byte[] key) => Get(CurrentReadOnlyContext, key);
public static ByteString? Get(ByteString key) => Get(CurrentReadOnlyContext, key);
public static ByteString? Get(byte[] key) => Get(CurrentReadOnlyContext, key);
public static void Put(ByteString key, ByteString value) => Put(CurrentContext, key, value);
public static void Put(byte[] key, ByteString value) => Put(CurrentContext, key, value);
public static void Put(byte[] key, byte[] value) => Put(CurrentContext, key, value);
Expand Down
16 changes: 8 additions & 8 deletions src/Neo.SmartContract.Framework/Services/StorageMap.cs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ public class StorageMap
private readonly StorageContext context;
private readonly byte[] prefix;

public extern ByteString this[ByteString key]
public extern ByteString? this[ByteString key]
{
[CallingConvention(CallingConvention.Cdecl)]
[OpCode(OpCode.UNPACK)]
Expand All @@ -43,7 +43,7 @@ public extern ByteString this[ByteString key]
set;
}

public extern ByteString this[byte[] key]
public extern ByteString? this[byte[] key]
{
[CallingConvention(CallingConvention.Cdecl)]
[OpCode(OpCode.UNPACK)]
Expand Down Expand Up @@ -112,7 +112,7 @@ public extern ByteString this[byte[] key]
[OpCode(OpCode.CAT)]
[OpCode(OpCode.SWAP)]
[Syscall("System.Storage.Get")]
public extern ByteString Get(ByteString key);
public extern ByteString? Get(ByteString key);

[CallingConvention(CallingConvention.Cdecl)]
[OpCode(OpCode.UNPACK)]
Expand Down Expand Up @@ -204,7 +204,7 @@ public extern ByteString this[byte[] key]
[OpCode(OpCode.CAT)]
[OpCode(OpCode.SWAP)]
[Syscall("System.Storage.Get")]
public extern ByteString Get(byte[] key);
public extern ByteString? Get(byte[] key);

[CallingConvention(CallingConvention.Cdecl)]
[OpCode(OpCode.UNPACK)]
Expand Down Expand Up @@ -289,16 +289,16 @@ public extern ByteString this[byte[] key]
[OpCode(OpCode.CONVERT, StackItemType.Integer)]
public extern BigInteger GetIntegerOrZero(byte[] key);

public object GetObject(ByteString key)
public object? GetObject(ByteString key)
{
ByteString value = Get(key);
ByteString? value = Get(key);
if (value is null) return null;
return StdLib.Deserialize(value);
}

public object GetObject(byte[] key)
public object? GetObject(byte[] key)
{
ByteString value = Get(key);
ByteString? value = Get(key);
if (value is null) return null;
return StdLib.Deserialize(value);
}
Expand Down
38 changes: 26 additions & 12 deletions src/Neo.SmartContract.Framework/TokenContract.cs
Original file line number Diff line number Diff line change
Expand Up @@ -17,35 +17,49 @@ namespace Neo.SmartContract.Framework
{
public abstract class TokenContract : SmartContract
{
protected const byte Prefix_TotalSupply = 0x00;
protected const byte Prefix_Balance = 0x01;
protected static readonly ByteString Prefix_TotalSupply = "\x00";
shargon marked this conversation as resolved.
Show resolved Hide resolved
protected static readonly ByteString Prefix_Balance = "\x01";

public abstract string Symbol { [Safe] get; }

public abstract byte Decimals { [Safe] get; }

[Stored(Prefix_TotalSupply)]
public static BigInteger TotalSupply { [Safe] get; protected set; }
Copy link
Member

Choose a reason for hiding this comment

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

Why avoid the Stored?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why avoid the Stored?

I think it was because Stored does not support a ByteString prefix, but I actually forgot why. I will find the reason.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why avoid the Stored?

Yes it is because ByteString is not supported by Stored

public static BigInteger TotalSupply
{
[Safe]
get
{
ByteString? stored = Storage.Get(Prefix_TotalSupply);
return (BigInteger)stored!;
}
protected set
{
Storage.Put(Prefix_TotalSupply, value);
}
}

[Safe]
public static BigInteger BalanceOf(UInt160 owner)
{
if (owner is null || !owner.IsValid)
throw new Exception("The argument \"owner\" is invalid.");
StorageMap balanceMap = new(Storage.CurrentContext, Prefix_Balance);
return (BigInteger)balanceMap[owner];
ByteString? balanceStored = Storage.Get(Prefix_Balance + owner);
return (BigInteger)balanceStored!;
}

protected static bool UpdateBalance(UInt160 owner, BigInteger increment)
{
StorageMap balanceMap = new(Storage.CurrentContext, Prefix_Balance);
Copy link
Member

Choose a reason for hiding this comment

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

It's better to use Storage map, it's more secure, less humans erros using concat keys

Copy link
Contributor Author

Choose a reason for hiding this comment

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

StorageMap costs much more GAS. But I am ok to revert it to StorageMap

BigInteger balance = (BigInteger)balanceMap[owner];
StorageContext currentContext = Storage.CurrentContext;
ByteString ownerKey = Prefix_Balance + owner;
ByteString? balanceStored = Storage.Get(currentContext, ownerKey);
BigInteger balance = (BigInteger)balanceStored!;
balance += increment;
if (balance < 0) return false;
if (balance.IsZero)
balanceMap.Delete(owner);
if (balance < 0)
return false;
if (Helper.NumEqual(balance, 0))
Storage.Delete(currentContext, ownerKey);
else
balanceMap.Put(owner, balance);
Storage.Put(currentContext, ownerKey, balance);
return true;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ private static bool IsOwner() =>

public static void SetOwner(UInt160 newOwner)
{
if (IsOwner() == false)
if (!IsOwner())
throw new InvalidOperationException("No Authorization!");

ExecutionEngine.Assert(newOwner.IsValid && !newOwner.IsZero, "owner must be valid");
Expand All @@ -60,14 +60,14 @@ public static void SetOwner(UInt160 newOwner)

public static new void Burn(UInt160 account, BigInteger amount)
{
if (IsOwner() == false)
Copy link
Contributor

@Jim8y Jim8y Oct 27, 2024

Choose a reason for hiding this comment

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

actually == false is a better format as it is reader friendly, but anyway. i dont mind.

if (!IsOwner())
throw new InvalidOperationException("No Authorization!");
Nep17Token.Burn(account, amount);
}

public static new void Mint(UInt160 to, BigInteger amount)
{
if (IsOwner() == false)
if (!IsOwner())
throw new InvalidOperationException("No Authorization!");
Nep17Token.Mint(to, amount);
}
Expand Down Expand Up @@ -109,7 +109,7 @@ public static void _deploy(object data, bool update)

public static void Update(ByteString nefFile, string manifest, object? data = null)
{
if (IsOwner() == false)
if (!IsOwner())
throw new InvalidOperationException("No authorization.");
ContractManagement.Update(nefFile, manifest, data);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ private static bool IsOwner() =>

public static void SetOwner(UInt160 newOwner)
{
if (IsOwner() == false)
if (!IsOwner())
throw new InvalidOperationException("No Authorization!");

ExecutionEngine.Assert(newOwner.IsValid && !newOwner.IsZero, "owner must be valid");
Expand Down Expand Up @@ -78,7 +78,7 @@ public static void _deploy(object data, bool update)

public static void Update(ByteString nefFile, string manifest, object? data = null)
{
if (IsOwner() == false)
if (!IsOwner())
throw new InvalidOperationException("No authorization.");
ContractManagement.Update(nefFile, manifest, data);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,8 @@ public virtual void TestSetGetOwner()
Engine.SetTransactionSigners(Alice);
Assert.ThrowsException<TestException>(() => Contract.Owner = UInt160.Zero);
var exception = Assert.ThrowsException<TestException>(() => Contract.Owner = InvalidUInt160.Null);
Assert.IsInstanceOfType<InvalidOperationException>(exception.InnerException);
// not InvalidOperationExcpetion, because no SIZE operation on null
Assert.IsInstanceOfType<Exception>(exception.InnerException);
Assert.ThrowsException<TestException>(() => Contract.Owner = InvalidUInt160.InvalidLength);
Assert.ThrowsException<TestException>(() => Contract.Owner = InvalidUInt160.InvalidType);

Expand Down
Loading
Loading