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.

Sign up for GitHub

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 candidate events #2754

Merged
merged 9 commits into from
May 26, 2022
Merged

Add candidate events #2754

Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
9eef2ca
Add events
shargon May 23, 2022
f040db1
Unify
shargon May 24, 2022
703d704
Distribute gas after notification
erikzhang May 24, 2022
a80fa80
Erik's feedback
shargon May 26, 2022
de0412e
Optimize without neo
shargon May 26, 2022
744a1d7
Update src/neo/SmartContract/Native/NeoToken.cs
shargon May 26, 2022
23f55c5
Merge branch 'master' into candidate-events
shargon May 26, 2022
08c8605
Fix UT
erikzhang May 26, 2022
f495f20
Merge branch 'master' into candidate-events
erikzhang May 26, 2022
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
76 changes: 76 additions & 0 deletions src/neo/SmartContract/Native/NeoToken.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
using Neo.IO;
using Neo.Persistence;
using Neo.SmartContract.Iterators;
using Neo.SmartContract.Manifest;
using Neo.VM;
using Neo.VM.Types;
using System;
Expand Down Expand Up @@ -56,6 +57,73 @@ public sealed class NeoToken : FungibleToken<NeoToken.NeoAccountState>
internal NeoToken()
{
this.TotalAmount = 100000000 * Factor;

var events = new List<ContractEventDescriptor>(Manifest.Abi.Events)
{
new ContractEventDescriptor
{
Name = "CandidateRegistered",
Parameters = new ContractParameterDefinition[]
{
new ContractParameterDefinition()
{
Name = "pubkey",
Type = ContractParameterType.PublicKey
},
new ContractParameterDefinition()
{
Name = "votes",
Type = ContractParameterType.Integer
}
}
},
new ContractEventDescriptor
{
Name = "CandidateUnregistered",
Copy link
Member

Choose a reason for hiding this comment

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

Merge it with CandidateRegistered? Maybe CandidateStateChanged or something.

Copy link
Contributor

Choose a reason for hiding this comment

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

Explicit events seems to match closer to existing events in the system. For example; we have Update, Destroy and Deploy instead of ContractStateChanged with a New/Updated/Destroyed parameter.

Copy link
Member

Choose a reason for hiding this comment

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

If we have Register and Unregister, then we need Vote and Unvote.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd be totally fine with that too. However, we do not have Burn or Mint either. We just have Transfer with a Null to or from respectively. It's not like everything needs to be (or even is) symmetrical in the system.

Copy link
Member

Choose a reason for hiding this comment

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

Then I prefer to merge Register and Unregister.🤭

Copy link
Member Author

Choose a reason for hiding this comment

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

Unify is always good

Copy link
Contributor

Choose a reason for hiding this comment

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

Unifying just makes post processing "harder" where explicit you can switch on the event name.

Then I prefer to merge Register and Unregister.🤭

I was trying to point out that vote could work similar to the transfer event. How about we vote for it

🎉 for all explicit (CandidateRegistered, CandidateUnregistered, Vote, Unvote)
❤️ for implicit (CandidateStateChanged with extra param for Registered/Unregistered state, vote with no extra parameter and the consumer parses the to field to determine if it is a vote or unvote)

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd definitely like to see one Vote event. But for some reason I also like two events for candidate registration/unregistration. Go figure. Maybe that's because we have one vote method and a pair of registerCandidate/unregisterCandidate. At the same time a pair of methods doesn't mean we need to have a pair of events.

Copy link
Contributor

Choose a reason for hiding this comment

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

My first choice would have been registerCandidate/unregisterCandidate and a single vote, pretty much how it was PR'ed. But Erik seems to want all explicit or all implicit hence the 2 choices above 🤷‍♂️

Parameters = new ContractParameterDefinition[]
{
new ContractParameterDefinition()
{
Name = "pubkey",
Type = ContractParameterType.PublicKey
},
new ContractParameterDefinition()
{
Name = "votes",
Type = ContractParameterType.Integer
}
}
},
new ContractEventDescriptor
{
Name = "VoteCasted",
erikzhang marked this conversation as resolved.
Show resolved Hide resolved
Parameters = new ContractParameterDefinition[]
{
new ContractParameterDefinition()
{
Name = "account",
Type = ContractParameterType.Hash160
},
new ContractParameterDefinition()
{
Name = "voteTo",
Type = ContractParameterType.PublicKey
},
new ContractParameterDefinition()
{
Name = "balance",
Type = ContractParameterType.Integer
},
new ContractParameterDefinition()
{
Name = "votes",
Type = ContractParameterType.Integer
}
}
}
};

Manifest.Abi.Events = events.ToArray();
}

public override BigInteger TotalSupply(DataCache snapshot)
Expand Down Expand Up @@ -295,7 +363,10 @@ private bool RegisterCandidate(ApplicationEngine engine, ECPoint pubkey)
StorageKey key = CreateStorageKey(Prefix_Candidate).Add(pubkey);
StorageItem item = engine.Snapshot.GetAndChange(key, () => new StorageItem(new CandidateState()));
CandidateState state = item.GetInteroperable<CandidateState>();
if (state.Registered) return true;
state.Registered = true;
engine.SendNotification(Hash, "CandidateRegistered",
new VM.Types.Array(engine.ReferenceCounter) { pubkey.ToArray(), state.Votes });
return true;
}

Expand All @@ -308,8 +379,11 @@ private bool UnregisterCandidate(ApplicationEngine engine, ECPoint pubkey)
if (engine.Snapshot.TryGet(key) is null) return true;
StorageItem item = engine.Snapshot.GetAndChange(key);
CandidateState state = item.GetInteroperable<CandidateState>();
if (!state.Registered) return true;
state.Registered = false;
CheckCandidate(engine.Snapshot, pubkey, state);
engine.SendNotification(Hash, "CandidateUnregistered",
new VM.Types.Array(engine.ReferenceCounter) { pubkey.ToArray(), state.Votes });
return true;
}

Expand Down Expand Up @@ -350,6 +424,8 @@ private async ContractTask<bool> Vote(ApplicationEngine engine, UInt160 account,
}
if (gasDistribution is not null)
await GAS.Mint(engine, gasDistribution.Account, gasDistribution.Amount, true);
engine.SendNotification(Hash, "VoteCasted",
new VM.Types.Array(engine.ReferenceCounter) { account.ToArray(), voteTo?.ToArray() ?? StackItem.Null, state_account.Balance, validator_new?.Votes ?? StackItem.Null });
return true;
}

Expand Down