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 Oracle PostPersist #2074

Merged
merged 24 commits into from
Nov 27, 2020
Merged
Show file tree
Hide file tree
Changes from 23 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
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
20 changes: 19 additions & 1 deletion src/neo/Ledger/TransactionVerificationContext.cs
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,16 @@ public class TransactionVerificationContext
/// </summary>
private readonly Dictionary<UInt160, BigInteger> senderFee = new Dictionary<UInt160, BigInteger>();

/// <summary>
/// Store oracle responses
/// </summary>
private readonly Dictionary<ulong, UInt256> oracleResponses = new Dictionary<ulong, UInt256>();

public void AddTransaction(Transaction tx)
Copy link
Member Author

@shargon shargon Nov 26, 2020

Choose a reason for hiding this comment

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

Why don't remove CheckTransaction and Change Add to bool TryAdd (thread-safe)?

Copy link
Member

Choose a reason for hiding this comment

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

Because CheckTransaction is called in Transaction.Verify().

{
var oracle = tx.GetAttribute<OracleResponse>();
if (oracle != null) oracleResponses.Add(oracle.Id, tx.Hash);

if (senderFee.TryGetValue(tx.Sender, out var value))
senderFee[tx.Sender] = value + tx.SystemFee + tx.NetworkFee;
else
Expand All @@ -25,13 +33,23 @@ public bool CheckTransaction(Transaction tx, StoreView snapshot)
{
BigInteger balance = NativeContract.GAS.BalanceOf(snapshot, tx.Sender);
senderFee.TryGetValue(tx.Sender, out var totalSenderFeeFromPool);

BigInteger fee = tx.SystemFee + tx.NetworkFee + totalSenderFeeFromPool;
return balance >= fee;
if (balance < fee) return false;

var oracle = tx.GetAttribute<OracleResponse>();
if (oracle != null && oracleResponses.ContainsKey(oracle.Id))
return false;
Copy link
Contributor

Choose a reason for hiding this comment

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

BTW, this can be replaced with generic "Conflicts" mechanism (#1991) that would also allow for proper transaction to have higher priority over fallback.


return true;
}

public void RemoveTransaction(Transaction tx)
{
if ((senderFee[tx.Sender] -= tx.SystemFee + tx.NetworkFee) == 0) senderFee.Remove(tx.Sender);

var oracle = tx.GetAttribute<OracleResponse>();
if (oracle != null) oracleResponses.Remove(oracle.Id);
}
}
}
5 changes: 3 additions & 2 deletions src/neo/SmartContract/Native/Oracle/OracleContract.cs
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,8 @@ protected override void PostPersist(ApplicationEngine engine)

//Remove the request from storage
StorageKey key = CreateStorageKey(Prefix_Request).Add(response.Id);
OracleRequest request = engine.Snapshot.Storages[key].GetInteroperable<OracleRequest>();
OracleRequest request = engine.Snapshot.Storages.TryGet(key)?.GetInteroperable<OracleRequest>();
if (request == null) continue;
engine.Snapshot.Storages.Delete(key);

//Remove the id from IdList
Expand All @@ -157,7 +158,7 @@ protected override void PostPersist(ApplicationEngine engine)
if (list.Count == 0) engine.Snapshot.Storages.Delete(key);

//Mint GAS for oracle nodes
nodes ??= NativeContract.Designate.GetDesignatedByRole(engine.Snapshot, Role.Oracle, engine.Snapshot.PersistingBlock.Index).Select(p => (Contract.CreateSignatureRedeemScript(p).ToScriptHash(), BigInteger.Zero)).ToArray();
nodes ??= Designate.GetDesignatedByRole(engine.Snapshot, Role.Oracle, engine.Snapshot.PersistingBlock.Index).Select(p => (Contract.CreateSignatureRedeemScript(p).ToScriptHash(), BigInteger.Zero)).ToArray();
if (nodes.Length > 0)
{
int index = (int)(response.Id % (ulong)nodes.Length);
Expand Down
23 changes: 23 additions & 0 deletions tests/neo.UnitTests/Ledger/UT_TransactionVerificationContext.cs
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,29 @@ private Transaction CreateTransactionWithFee(long networkFee, long systemFee)
return mock.Object;
}

[TestMethod]
public void TestDuplicateOracle()
{
// Fake balance
var snapshot = Blockchain.Singleton.GetSnapshot();

ApplicationEngine engine = ApplicationEngine.Create(TriggerType.Application, null, snapshot, long.MaxValue);
BigInteger balance = NativeContract.GAS.BalanceOf(snapshot, UInt160.Zero);
NativeContract.GAS.Burn(engine, UInt160.Zero, balance);
NativeContract.GAS.Mint(engine, UInt160.Zero, 8);

// Test
TransactionVerificationContext verificationContext = new TransactionVerificationContext();
var tx = CreateTransactionWithFee(1, 2);
tx.Attributes = new TransactionAttribute[] { new OracleResponse() { Code = OracleResponseCode.ConsensusUnreachable, Id = 1, Result = new byte[0] } };
verificationContext.CheckTransaction(tx, snapshot).Should().BeTrue();
verificationContext.AddTransaction(tx);

tx = CreateTransactionWithFee(2, 1);
tx.Attributes = new TransactionAttribute[] { new OracleResponse() { Code = OracleResponseCode.ConsensusUnreachable, Id = 1, Result = new byte[0] } };
verificationContext.CheckTransaction(tx, snapshot).Should().BeFalse();
}

[TestMethod]
public void TestTransactionSenderFee()
{
Expand Down