-
Notifications
You must be signed in to change notification settings - Fork 100
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
Add notary service #691
Add notary service #691
Conversation
System.ActorSystem.EventStream.Subscribe(notary, typeof(Blockchain.PersistCompleted)); | ||
System.ActorSystem.EventStream.Subscribe(notary, typeof(Blockchain.RelayResult)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Move to NotaryService
constructor, use
neoSystem.ActorSystem.EventStream.Subscribe(Self, typeof(Blockchain.PersistCompleted));
neoSystem.ActorSystem.EventStream.Subscribe(Self, typeof(Blockchain.RelayResult));
Co-authored-by: ZhangTao <ztgoforit@163.com>
Co-authored-by: ZhangTao <ztgoforit@163.com>
src/NotaryService/NotaryService.cs
Outdated
private bool started; | ||
|
||
private readonly ConcurrentDictionary<UInt256, NotaryTask> pendingQueue = new ConcurrentDictionary<UInt256, NotaryTask>(); | ||
private readonly Queue<NotaryRequest> payloadCache = new Queue<NotaryRequest>(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You use a simple queue to store and relay NotaryRequest
payloads. This scheme allows some malicious clients to easily spam the network with useless (and cheap) NotaryRequest
payloads. To prevent such behaviour we need to range payloads using all of the transaction mempool rules with some additional logic.
Please, take a look at reference, we use separate notaryRequestPool
to store NotaryRequest
payloads. In this mempool the payloads are being ranged by fallback's fees and/or attributes. To accept the incoming NotaryRequest
payload we need to ensure that payer (which is the second fallback's signer) has enough funds deposited to Notary contract to pay for all fallback transactions that are currently in the notaryRequestPool. We also need to ensure that incoming NotaryRequest
's fallback tx doesn't conflict with mempooled NotaryRequests. And several other checks need to be done (very similar to simple tx processing).
To support the described scheme we extended our transaction mempool in two ways:
- It accepts
payerIndex
in the constructor (see the code). For standard completed transactionspayerIndex
is 0 (sender of transaction should pay the fees). For incompleted fallback transactions of incoming NotaryRequestspayerIndex
is 1 (fallback's first signer is Notary contract, and the second signer is the sender ofNotaryRequest
, so he should deposit funds to Notary). - It uses two different balance checkers (aka
mempool.Feer
, see the code and the interface itself). For standard completed transactionsFeer
returns sender's GAS balance is enough to pay the fees. For incompleted fallback transactionsFeer
returns Notary deposit balance of theNotaryRequest
sender (see fee.GetUtilityTokenBalance(payer)).
So we need to refactor the Queue<NotaryRequest> payloadCache
logic with regard to this comment (maybe replace it with modified Mempool).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See doubiliu#2
Add notary request pool
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Somehow we need to write tests for this code or for separate parts of it. We need to be sure that a set of different notary requests can be properly handled, signatures can be properly extracted and transactions can be properly completed.
public int Compare(NotaryRequest x, NotaryRequest y) | ||
{ | ||
var r = x.FallbackTransaction.FeePerByte.CompareTo(y.FallbackTransaction.FeePerByte); | ||
if (r != 0) return r; | ||
r = x.FallbackTransaction.NetworkFee.CompareTo(y.FallbackTransaction.NetworkFee); | ||
if (r != 0) return r; | ||
return x.FallbackTransaction.Hash.CompareTo(y.FallbackTransaction.Hash); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fallback transactions should be compared like any standard mempooled transactions, so we need to consider HighPriority
attribute.
removed = null; | ||
if (requests.ContainsKey(request.FallbackTransaction.Hash)) return false; | ||
if (!request.VerifyStateDependent(neoSystem.Settings, neoSystem.StoreView)) return false; | ||
if (!context.CheckFallback(request.FallbackTransaction, neoSystem.StoreView)) return false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Imagine the situation when user has 1 GAS deposited to notary contract and one notary request in the pool that costs 0.8 GAS. If the user sends more prioritised notary request (i.e. request with 0.9 GAS fee), then the old request should be kicked off. This check doesn't perform this. So probably, this check should be done after CheckConflicts
and take into account requests that should be removed from the pool.
private bool CheckConflicts(NotaryRequest request) | ||
{ | ||
var fallbackTx = request.FallbackTransaction; | ||
var mainTx = request.MainTransaction; | ||
foreach (var r in tasks.Values) | ||
{ | ||
var tx = requests[r.First()].MainTransaction; | ||
foreach (var conflict in tx.GetAttributes<ConflictAttribute>()) | ||
{ | ||
if (conflict.Hash == fallbackTx.Hash && tx.Signers.Any(p => p.Account == fallbackTx.Signers[1].Account)) return false; | ||
if (conflict.Hash == mainTx.Hash && tx.Signers.Any(p => p.Account == mainTx.Sender)) return false; | ||
} | ||
} | ||
return true; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Check conflicts logic should be exactly the same as the standard mempool has in neo-project/neo#2661.
if (!r.IsSent && r.IsMainCompleted() && r.MinNotValidBefore > currHeight) | ||
{ | ||
Finalize(r.MainTx); | ||
r.IsSent = true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Request shouldn't be mark as sent until main transaction appears in the subsequent block. Otherwise resending may be required.
foreach (var fb in r.FallbackTxs) | ||
{ | ||
var nvb = fb.GetAttributes<NotValidBefore>().ToArray()[0].Height; | ||
if (nvb <= currHeight) Finalize(fb); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure, do you have a way to avoid re-finalisation of the same fallback? fb
is not removed from the r.FallbackTxs
list after finalisation.
} | ||
if (tx.Witnesses[i].InvocationScript.Length != 0) | ||
{ | ||
if (tx.Witnesses[i].InvocationScript.Length != 66 || (tx.Witnesses[i].InvocationScript[0] != (byte)OpCode.PUSHDATA1 && tx.Witnesses[i].InvocationScript[1] != 64)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of
(tx.Witnesses[i].InvocationScript[0] != (byte)OpCode.PUSHDATA1 && tx.Witnesses[i].InvocationScript[1] != 64)
should be
(tx.Witnesses[i].InvocationScript[0] != (byte)OpCode.PUSHDATA1 || tx.Witnesses[i].InvocationScript[1] != 64)
{ | ||
Typ = RequestType.Signature, | ||
NSigsLeft = 1, | ||
Pubs = new ECPoint[pubs.Length], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is that? Standard signature witness has the single public key binded to it, and the key can be retrieved from verification script, so don't use pubs
here.
if (r.IsMainCompleted() && r.MinNotValidBefore > currentHeight) | ||
{ | ||
Finalize(r.MainTx); | ||
r.IsSent = true; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be done after the whole set of main transaction witnesses is processed (outside for
loop).
if (r.IsMainCompleted() && r.MinNotValidBefore > currentHeight) | ||
{ | ||
Finalize(r.MainTx); | ||
r.IsSent = true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The same note about isSent
, this field should be updated only after main tx persisting.
@doubiliu please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.
Contributor License AgreementContribution License AgreementThis Contribution License Agreement ( “Agreement” ) is agreed to by the party signing below ( “You” ), 1. Definitions. “Code” means the computer software code, whether in human-readable or machine-executable form, “Project” means any of the projects owned or managed by .NET Foundation and offered under a license “Submit” is the act of uploading, submitting, transmitting, or distributing code or other content to any “Submission” means the Code and any other copyrightable material Submitted by You, including any 2. Your Submission. You must agree to the terms of this Agreement before making a Submission to any 3. Originality of Work. You represent that each of Your Submissions is entirely Your 4. Your Employer. References to “employer” in this Agreement include Your employer or anyone else 5. Licenses. a. Copyright License. You grant .NET Foundation, and those who receive the Submission directly b. Patent License. You grant .NET Foundation, and those who receive the Submission directly or c. Other Rights Reserved. Each party reserves all rights not expressly granted in this Agreement. 6. Representations and Warranties. You represent that You are legally entitled to grant the above 7. Notice to .NET Foundation. You agree to notify .NET Foundation in writing of any facts or 8. Information about Submissions. You agree that contributions to Projects and information about 9. Governing Law/Jurisdiction. This Agreement is governed by the laws of the State of Washington, and 10. Entire Agreement/Assignment. This Agreement is the entire agreement between the parties, and .NET Foundation dedicates this Contribution License Agreement to the public domain according to the Creative Commons CC0 1. |
close as expired and no longer being maintained. |
close #2646
This is the second part of the code for the notary service