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

Payloads: add NotValidBefore transaction attribute #2812

Merged
merged 3 commits into from
Mar 11, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
46 changes: 46 additions & 0 deletions src/Neo/Network/P2P/Payloads/NotValidBefore.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
using Neo.IO;
using Neo.Json;
using Neo.Persistence;
using Neo.SmartContract.Native;
using System.IO;

namespace Neo.Network.P2P.Payloads
{
public class NotValidBefore : TransactionAttribute
{
/// <summary>
/// Indicates that the transaction is not valid before this height.
/// </summary>
public uint Height;

public override TransactionAttributeType Type => TransactionAttributeType.NotValidBefore;

public override bool AllowMultiple => false;

public override int Size => base.Size +
sizeof(uint); // Height.

protected override void DeserializeWithoutType(ref MemoryReader reader)
{
Height = reader.ReadUInt32();
}

protected override void SerializeWithoutType(BinaryWriter writer)
{
writer.Write(Height);
}

public override JObject ToJson()
{
JObject json = base.ToJson();
json["height"] = Height;
return json;
}

public override bool Verify(DataCache snapshot, Transaction tx)
{
var block_height = NativeContract.Ledger.CurrentIndex(snapshot);
return block_height >= Height;
Copy link
Member

Choose a reason for hiding this comment

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

It wont wait in the mempool until it's valid

Copy link
Member

Choose a reason for hiding this comment

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

That's the desired behaviour, the transaction is not supposed to wait. It's the sender's responsibility to send it at the right time.

Copy link
Member

Choose a reason for hiding this comment

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

Then... why we need it? the sender can sign it after the desired Height.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

#1992:

It can be used to create some transactions in advance with a guarantee that they won't be accepted until specified block, that property will be used for signature collection service (#1573).

In #1573 context this allows to have some "hostage" transaction that can be used if the main one is not completed in time (which works especially nice with #1991). But in general, any scenario where you can/need to create a transaction now, but you don't want it to be used until some block. Some offline signing scenarios might benefit from it as well.

Copy link
Member

Choose a reason for hiding this comment

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

If the transaction is not valid, it will be included in the know hashes list and it won't be valid until an unknown time

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's not supposed to ever appear on the P2P network (unless it's wrapped into P2PNotaryPayload) until it becomes valid. It can't really appear there before NVB height, any node will reject it right away, so I doubt anyone will even try sending it.

Copy link
Member

Choose a reason for hiding this comment

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

Maybe it's a different problem, but what will happend if it's in known hashes and now it's valid?

How we will know that it was rejected because previously was rejected?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  1. Ordinary transactions are not that much different wrt this problem, if node A sends to node B a tx T that for example is lacking some signatures, then it's gonna be the same after signatures are added to T.
  2. This actually looks like a C# implementation problem as I can leverage it to prevent transaction dissemination through the network, a malicious node can specifically strip witnesses from a valid tx and send it to all peers. The result will depend on the sequence of events, if there are enough bad nodes and if they're quick enough they can effectively block the tx.

}
}
}
8 changes: 7 additions & 1 deletion src/Neo/Network/P2P/Payloads/TransactionAttributeType.cs
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,12 @@ public enum TransactionAttributeType : byte
/// Indicates that the transaction is an oracle response.
/// </summary>
[ReflectionCache(typeof(OracleResponse))]
OracleResponse = 0x11
OracleResponse = 0x11,

/// <summary>
/// Indicates that the transaction is not valid before <see cref="NotValidBefore.Height"/>.
/// </summary>
[ReflectionCache(typeof(NotValidBefore))]
NotValidBefore = 0x20
}
}
75 changes: 75 additions & 0 deletions tests/Neo.UnitTests/Network/P2P/Payloads/UT_NotValidBefore.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
using FluentAssertions;
using Microsoft.VisualStudio.TestTools.UnitTesting;
using Neo.IO;
using Neo.Network.P2P.Payloads;
using Neo.SmartContract.Native;
using System;

namespace Neo.UnitTests.Network.P2P.Payloads
{
[TestClass]
public class UT_NotValidBefore
{
[TestMethod]
public void Size_Get()
{
var test = new NotValidBefore();
test.Size.Should().Be(5);
}

[TestMethod]
public void ToJson()
{
var test = new NotValidBefore();
test.Height = 42;
var json = test.ToJson().ToString();
Assert.AreEqual(@"{""type"":""NotValidBefore"",""height"":42}", json);
}

[TestMethod]
public void DeserializeAndSerialize()
{
var test = new NotValidBefore();

var clone = test.ToArray().AsSerializable<NotValidBefore>();
Assert.AreEqual(clone.Type, test.Type);

// As transactionAttribute

byte[] buffer = test.ToArray();
var reader = new MemoryReader(buffer);
clone = TransactionAttribute.DeserializeFrom(ref reader) as NotValidBefore;
Assert.AreEqual(clone.Type, test.Type);

// Wrong type

buffer[0] = 0xff;
reader = new MemoryReader(buffer);
try
{
TransactionAttribute.DeserializeFrom(ref reader);
Assert.Fail();
}
catch (FormatException) { }
reader = new MemoryReader(buffer);
try
{
new NotValidBefore().Deserialize(ref reader);
Assert.Fail();
}
catch (FormatException) { }
}

[TestMethod]
public void Verify()
{
var test = new NotValidBefore();
var snapshot = TestBlockchain.GetTestSnapshot();
test.Height = NativeContract.Ledger.CurrentIndex(snapshot) + 1;

Assert.IsFalse(test.Verify(snapshot, new Transaction()));
test.Height--;
Assert.IsTrue(test.Verify(snapshot, new Transaction()));
}
}
}