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

Improved scheme for Conflicts attribute storing and pricing #2913

Merged
merged 15 commits into from
Nov 8, 2023
Merged
Show file tree
Hide file tree
Changes from 13 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
2 changes: 1 addition & 1 deletion src/Neo/Ledger/Blockchain.cs
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ private void OnFillMemoryPool(IEnumerable<Transaction> transactions)
{
if (NativeContract.Ledger.ContainsTransaction(snapshot, tx.Hash))
continue;
if (NativeContract.Ledger.ContainsConflictHash(snapshot, tx.Hash, tx.Signers.Select(s => s.Account)))
if (NativeContract.Ledger.ContainsConflictHash(snapshot, tx.Hash, tx.Signers.Select(s => s.Account), system.Settings.MaxTraceableBlocks))
continue;
// First remove the tx if it is unverified in the pool.
system.MemPool.TryRemoveUnVerified(tx.Hash, out _);
Expand Down
2 changes: 1 addition & 1 deletion src/Neo/NeoSystem.cs
Original file line number Diff line number Diff line change
Expand Up @@ -287,7 +287,7 @@ public bool ContainsTransaction(UInt256 hash)
/// <returns><see langword="true"/> if the transaction conflicts with on-chain transaction; otherwise, <see langword="false"/>.</returns>
public bool ContainsConflictHash(UInt256 hash, IEnumerable<UInt160> signers)
{
return NativeContract.Ledger.ContainsConflictHash(StoreView, hash, signers);
return NativeContract.Ledger.ContainsConflictHash(StoreView, hash, signers, Settings.MaxTraceableBlocks);
}
}
}
39 changes: 32 additions & 7 deletions src/Neo/SmartContract/Native/LedgerContract.cs
Original file line number Diff line number Diff line change
Expand Up @@ -47,13 +47,25 @@ internal override ContractTask OnPersist(ApplicationEngine engine)
engine.Snapshot.Add(CreateStorageKey(Prefix_Block).Add(engine.PersistingBlock.Hash), new StorageItem(Trim(engine.PersistingBlock).ToArray()));
foreach (TransactionState tx in transactions)
{
engine.Snapshot.Add(CreateStorageKey(Prefix_Transaction).Add(tx.Transaction.Hash), new StorageItem(tx));
// It's possible that there are previously saved malicious conflict records for this transaction.
// If so, then remove it and store the relevant transaction itself.
var txKey = CreateStorageKey(Prefix_Transaction).Add(tx.Transaction.Hash);
engine.Snapshot.Delete(txKey);
engine.Snapshot.Add(txKey, new StorageItem(tx));
shargon marked this conversation as resolved.
Show resolved Hide resolved

// Store transaction's conflicits.
var conflictingSigners = tx.Transaction.Signers.Select(s => s.Account);
foreach (var attr in tx.Transaction.GetAttributes<Conflicts>())
{
var conflictRecord = engine.Snapshot.GetAndChange(CreateStorageKey(Prefix_Transaction).Add(attr.Hash),
() => new StorageItem(new TransactionState { ConflictingSigners = Array.Empty<UInt160>() })).GetInteroperable<TransactionState>();
conflictRecord.ConflictingSigners = conflictRecord.ConflictingSigners.Concat(conflictingSigners).Distinct().ToArray();
var recordKey = CreateStorageKey(Prefix_Transaction).Add(attr.Hash);
engine.Snapshot.Delete(recordKey);
engine.Snapshot.Add(recordKey, new StorageItem(new TransactionState() { BlockIndex = engine.PersistingBlock.Index }));
foreach (var signer in conflictingSigners)
{
var conflictKey = CreateStorageKey(Prefix_Transaction).Add(attr.Hash).Add(signer);
engine.Snapshot.Delete(conflictKey);
engine.Snapshot.Add(conflictKey, new StorageItem(new TransactionState() { BlockIndex = engine.PersistingBlock.Index }));
}
}
}
engine.SetState(transactions);
Expand Down Expand Up @@ -145,11 +157,24 @@ public bool ContainsTransaction(DataCache snapshot, UInt256 hash)
/// <param name="snapshot">The snapshot used to read data.</param>
/// <param name="hash">The hash of the conflicting transaction.</param>
/// <param name="signers">The list of signer accounts of the conflicting transaction.</param>
/// <param name="maxTraceableBlocks">MaxTraceableBlocks protocol setting.</param>
/// <returns><see langword="true"/> if the blockchain contains the hash of the conflicting transaction; otherwise, <see langword="false"/>.</returns>
public bool ContainsConflictHash(DataCache snapshot, UInt256 hash, IEnumerable<UInt160> signers)
public bool ContainsConflictHash(DataCache snapshot, UInt256 hash, IEnumerable<UInt160> signers, uint maxTraceableBlocks)
{
var state = snapshot.TryGet(CreateStorageKey(Prefix_Transaction).Add(hash))?.GetInteroperable<TransactionState>();
return state is not null && state.Transaction is null && (signers is null || state.ConflictingSigners.Intersect(signers).Any());
// Check the dummy stub firstly to define whether there's exist at least one conflict record.
var stub = snapshot.TryGet(CreateStorageKey(Prefix_Transaction).Add(hash))?.GetInteroperable<TransactionState>();
if (stub is null || stub.Transaction is not null || !IsTraceableBlock(snapshot, stub.BlockIndex, maxTraceableBlocks))
return false;

// At least one conflict record is found, then need to check signers intersection.
foreach (var signer in signers)
{
var state = snapshot.TryGet(CreateStorageKey(Prefix_Transaction).Add(hash).Add(signer))?.GetInteroperable<TransactionState>();
if (state is not null && IsTraceableBlock(snapshot, state.BlockIndex, maxTraceableBlocks))
return true;
}

return false;
}

/// <summary>
Expand Down
17 changes: 7 additions & 10 deletions src/Neo/SmartContract/Native/TransactionState.cs
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,6 @@ public class TransactionState : IInteroperable
/// </summary>
public Transaction Transaction;

public UInt160[] ConflictingSigners;

/// <summary>
/// The execution state
/// </summary>
Expand All @@ -47,7 +45,6 @@ IInteroperable IInteroperable.Clone()
{
BlockIndex = BlockIndex,
Transaction = Transaction,
ConflictingSigners = ConflictingSigners,
State = State,
_rawTransaction = _rawTransaction
};
Expand All @@ -58,7 +55,6 @@ void IInteroperable.FromReplica(IInteroperable replica)
TransactionState from = (TransactionState)replica;
BlockIndex = from.BlockIndex;
Transaction = from.Transaction;
ConflictingSigners = from.ConflictingSigners;
State = from.State;
if (_rawTransaction.IsEmpty)
_rawTransaction = from._rawTransaction;
Expand All @@ -67,20 +63,21 @@ void IInteroperable.FromReplica(IInteroperable replica)
void IInteroperable.FromStackItem(StackItem stackItem)
{
Struct @struct = (Struct)stackItem;
if (@struct.Count == 1)
{
ConflictingSigners = ((VM.Types.Array)@struct[0]).Select(u => new UInt160(u.GetSpan())).ToArray();
return;
}
BlockIndex = (uint)@struct[0].GetInteger();

// Conflict record.
if (@struct.Count == 1) return;

// Fully-qualified transaction.
_rawTransaction = ((ByteString)@struct[1]).Memory;
Transaction = _rawTransaction.AsSerializable<Transaction>();
State = (VMState)(byte)@struct[2].GetInteger();
}

StackItem IInteroperable.ToStackItem(ReferenceCounter referenceCounter)
{
if (Transaction is null) return new Struct(referenceCounter) { new VM.Types.Array(referenceCounter, ConflictingSigners.Select(u => new ByteString(u.ToArray())).ToArray()) };
if (Transaction is null)
return new Struct(referenceCounter) { BlockIndex };
if (_rawTransaction.IsEmpty)
_rawTransaction = Transaction.ToArray();
return new Struct(referenceCounter) { BlockIndex, _rawTransaction, (byte)State };
Expand Down
23 changes: 18 additions & 5 deletions tests/Neo.UnitTests/Ledger/UT_Blockchain.cs
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ public void TestMaliciousOnChainConflict()
{
Header = new Header()
{
Index = 10000,
Index = 5, // allow tx1, tx2 and tx3 to fit into MaxValidUntilBlockIncrement.
MerkleRoot = UInt256.Zero,
NextConsensus = UInt160.Zero,
PrevHash = UInt256.Zero,
Expand All @@ -149,13 +149,26 @@ public void TestMaliciousOnChainConflict()
sb.EmitSysCall(ApplicationEngine.System_Contract_NativeOnPersist);
onPersistScript = sb.ToArray();
}
TransactionState[] transactionStates;
using (ApplicationEngine engine2 = ApplicationEngine.Create(TriggerType.OnPersist, null, snapshot, block, TestBlockchain.TheNeoSystem.Settings, 0))
{
engine2.LoadScript(onPersistScript);
if (engine2.Execute() != VMState.HALT) throw new InvalidOperationException();
Blockchain.ApplicationExecuted application_executed = new(engine2);
transactionStates = engine2.GetState<TransactionState[]>();
if (engine2.Execute() != VMState.HALT) throw engine2.FaultException;
engine2.Snapshot.Commit();
}
snapshot.Commit();

// Run PostPersist to update current block index in native Ledger.
// Relevant current block index is needed for conflict records checks.
byte[] postPersistScript;
using (ScriptBuilder sb = new())
{
sb.EmitSysCall(ApplicationEngine.System_Contract_NativePostPersist);
postPersistScript = sb.ToArray();
}
using (ApplicationEngine engine2 = ApplicationEngine.Create(TriggerType.PostPersist, null, snapshot, block, TestBlockchain.TheNeoSystem.Settings, 0))
{
engine2.LoadScript(postPersistScript);
if (engine2.Execute() != VMState.HALT) throw engine2.FaultException;
engine2.Snapshot.Commit();
}
snapshot.Commit();
Expand Down
68 changes: 0 additions & 68 deletions tests/Neo.UnitTests/Ledger/UT_MemoryPool.cs
Original file line number Diff line number Diff line change
Expand Up @@ -736,74 +736,6 @@ public void TestUpdatePoolForBlockPersisted()
_unit.VerifiedCount.Should().Be(0);
}


[TestMethod]
public async Task Malicious_OnChain_Conflict()
shargon marked this conversation as resolved.
Show resolved Hide resolved
{
// Arrange: prepare mempooled txs that have conflicts.
long txFee = 1;
var snapshot = GetSnapshot();
BigInteger balance = NativeContract.GAS.BalanceOf(snapshot, senderAccount);
ApplicationEngine engine = ApplicationEngine.Create(TriggerType.Application, null, snapshot, settings: TestBlockchain.TheNeoSystem.Settings, gas: long.MaxValue);
engine.LoadScript(Array.Empty<byte>());

var tx1 = CreateTransactionWithFeeAndBalanceVerify(txFee);
var tx2 = CreateTransactionWithFeeAndBalanceVerify(txFee);

tx1.Signers[0].Account = UInt160.Parse("0x0001020304050607080900010203040506070809"); // Different sender

await NativeContract.GAS.Mint(engine, tx1.Sender, 100000, true); // balance enough for all mempooled txs
await NativeContract.GAS.Mint(engine, tx2.Sender, 100000, true); // balance enough for all mempooled txs

tx1.Attributes = new TransactionAttribute[] { new Conflicts() { Hash = tx2.Hash } };

Assert.AreEqual(_unit.TryAdd(tx1, engine.Snapshot), VerifyResult.Succeed);

var block = new Block
{
Header = new Header()
{
Index = 10000,
MerkleRoot = UInt256.Zero,
NextConsensus = UInt160.Zero,
PrevHash = UInt256.Zero,
Witness = new Witness() { InvocationScript = Array.Empty<byte>(), VerificationScript = Array.Empty<byte>() }
},
Transactions = new Transaction[] { tx1 },
};
_unit.UpdatePoolForBlockPersisted(block, engine.Snapshot);

_unit.SortedTxCount.Should().Be(0);

// Simulate persist tx1

Assert.AreEqual(_unit.TryAdd(tx2, engine.Snapshot), VerifyResult.Succeed);

byte[] onPersistScript;
using (ScriptBuilder sb = new())
{
sb.EmitSysCall(ApplicationEngine.System_Contract_NativeOnPersist);
onPersistScript = sb.ToArray();
}

TransactionState[] transactionStates;
using (ApplicationEngine engine2 = ApplicationEngine.Create(TriggerType.OnPersist, null, engine.Snapshot, block, TestBlockchain.TheNeoSystem.Settings, 0))
{
engine2.LoadScript(onPersistScript);
if (engine2.Execute() != VMState.HALT) throw new InvalidOperationException();
Blockchain.ApplicationExecuted application_executed = new(engine2);
transactionStates = engine2.GetState<TransactionState[]>();
}

// Test tx2 arrive

snapshot.Commit();

var senderProbe = CreateTestProbe();
senderProbe.Send(TestBlockchain.TheNeoSystem.Blockchain, tx2);
senderProbe.ExpectMsg<Blockchain.RelayResult>(p => p.Result == VerifyResult.InsufficientFunds); // should be Succedded.
}

public static StorageKey CreateStorageKey(int id, byte prefix, byte[] key = null)
{
byte[] buffer = GC.AllocateUninitializedArray<byte>(sizeof(byte) + (key?.Length ?? 0));
Expand Down
9 changes: 2 additions & 7 deletions tests/Neo.UnitTests/Ledger/UT_TransactionState.cs
Original file line number Diff line number Diff line change
Expand Up @@ -36,11 +36,7 @@ public void Initialize()
};
originTrimmed = new TransactionState
{
ConflictingSigners = new UInt160[]
{
new UInt160(Crypto.Hash160(new byte[] { 1, 2, 3 })),
new UInt160(Crypto.Hash160(new byte[] { 4, 5, 6 }))
}
BlockIndex = 1,
};
}

Expand All @@ -67,10 +63,9 @@ public void TestDeserializeTrimmed()
TransactionState dest = new();
((IInteroperable)dest).FromStackItem(BinarySerializer.Deserialize(ref reader, ExecutionEngineLimits.Default, null));

dest.BlockIndex.Should().Be(0);
dest.BlockIndex.Should().Be(originTrimmed.BlockIndex);
dest.Transaction.Should().Be(null);
dest.Transaction.Should().BeNull();
CollectionAssert.AreEqual(originTrimmed.ConflictingSigners, dest.ConflictingSigners);
}
}
}