Skip to content

Refactor MainService.Vote class #4036

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

Merged
merged 12 commits into from
Jul 3, 2025
Merged

Refactor MainService.Vote class #4036

Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
3a042f7
Refactor code and improve styles
ajara87 Jun 29, 2025
8e6d3b8
Merge branch 'dev' into feature/vote-cli-clean
Jun 29, 2025
da63732
Update src/Neo.CLI/CLI/MainService.Vote.cs
Jun 30, 2025
b7f3b1a
Apply dotnet format to fix whitespace formatting
Jim8y Jun 30, 2025
b3cdd4e
Update src/Neo.CLI/CLI/MainService.Vote.cs
shargon Jun 30, 2025
d4b5d22
Merge branch 'dev' into feature/vote-cli-clean
cschuchardt88 Jun 30, 2025
5839f4f
Merge branch 'dev' into feature/vote-cli-clean
Wi1l-B0t Jun 30, 2025
fd8c6df
Merge branch 'dev' into feature/vote-cli-clean
Wi1l-B0t Jun 30, 2025
c28330a
Merge branch 'dev' into feature/vote-cli-clean
shargon Jun 30, 2025
e8b0509
Merge branch 'dev' into feature/vote-cli-clean
shargon Jul 1, 2025
0ed3ee0
Fix: add null arg to BuildNeoScript from UnVote. Rename BuildNativeSc…
ajara87 Jul 2, 2025
07b6793
Merge branch 'feature/vote-cli-clean' of https://github.com/ajara87/n…
ajara87 Jul 2, 2025
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
116 changes: 52 additions & 64 deletions src/Neo.CLI/CLI/MainService.Vote.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,17 @@

namespace Neo.CLI
{
public static class VoteMethods
{
public const string Register = "registerCandidate";
public const string Unregister = "unregisterCandidate";
public const string Vote = "vote";
public const string GetAccountState = "getAccountState";
public const string GetCandidates = "getCandidates";
public const string GetCommittee = "getCommittee";
public const string GetNextBlockValidators = "getNextBlockValidators";
}

partial class MainService
{
/// <summary>
Expand All @@ -35,30 +46,12 @@ private void OnRegisterCandidateCommand(UInt160 account)
{
var testGas = NativeContract.NEO.GetRegisterPrice(NeoSystem.StoreView) + (BigInteger)Math.Pow(10, NativeContract.GAS.Decimals) * 10;
if (NoWallet()) return;
WalletAccount currentAccount = CurrentWallet!.GetAccount(account);

if (currentAccount == null)
{
ConsoleHelper.Warning("This address isn't in your wallet!");
return;
}
else
{
if (currentAccount.Lock || currentAccount.WatchOnly)
{
ConsoleHelper.Warning("Locked or WatchOnly address.");
return;
}
}

ECPoint? publicKey = currentAccount.GetKey()?.PublicKey;
byte[] script;
using (ScriptBuilder scriptBuilder = new())
{
scriptBuilder.EmitDynamicCall(NativeContract.NEO.Hash, "registerCandidate", publicKey);
script = scriptBuilder.ToArray();
}
var currentAccount = GetValidAccountOrWarn(account);
if (currentAccount == null) return;

var publicKey = currentAccount.GetKey()?.PublicKey;
var script = BuildNeoScript(VoteMethods.Register, publicKey);
SendTransaction(script, account, (long)testGas);
}

Expand All @@ -70,30 +63,12 @@ private void OnRegisterCandidateCommand(UInt160 account)
private void OnUnregisterCandidateCommand(UInt160 account)
{
if (NoWallet()) return;
WalletAccount currentAccount = CurrentWallet!.GetAccount(account);

if (currentAccount == null)
{
ConsoleHelper.Warning("This address isn't in your wallet!");
return;
}
else
{
if (currentAccount.Lock || currentAccount.WatchOnly)
{
ConsoleHelper.Warning("Locked or WatchOnly address.");
return;
}
}

ECPoint? publicKey = currentAccount?.GetKey()?.PublicKey;
byte[] script;
using (ScriptBuilder scriptBuilder = new())
{
scriptBuilder.EmitDynamicCall(NativeContract.NEO.Hash, "unregisterCandidate", publicKey);
script = scriptBuilder.ToArray();
}
var currentAccount = GetValidAccountOrWarn(account);
if (currentAccount == null) return;

var publicKey = currentAccount?.GetKey()?.PublicKey;
var script = BuildNeoScript(VoteMethods.Unregister, publicKey);
SendTransaction(script, account);
}

Expand All @@ -106,13 +81,8 @@ private void OnUnregisterCandidateCommand(UInt160 account)
private void OnVoteCommand(UInt160 senderAccount, ECPoint publicKey)
{
if (NoWallet()) return;
byte[] script;
using (ScriptBuilder scriptBuilder = new())
{
scriptBuilder.EmitDynamicCall(NativeContract.NEO.Hash, "vote", senderAccount, publicKey);
script = scriptBuilder.ToArray();
}

var script = BuildNeoScript(VoteMethods.Vote, senderAccount, publicKey);
SendTransaction(script, senderAccount);
}

Expand All @@ -124,13 +94,8 @@ private void OnVoteCommand(UInt160 senderAccount, ECPoint publicKey)
private void OnUnvoteCommand(UInt160 senderAccount)
{
if (NoWallet()) return;
byte[] script;
using (ScriptBuilder scriptBuilder = new())
{
scriptBuilder.EmitDynamicCall(NativeContract.NEO.Hash, "vote", senderAccount, null);
script = scriptBuilder.ToArray();
}

var script = BuildNeoScript(VoteMethods.Vote, senderAccount, null);
SendTransaction(script, senderAccount);
}

Expand All @@ -140,7 +105,7 @@ private void OnUnvoteCommand(UInt160 senderAccount)
[ConsoleCommand("get candidates", Category = "Vote Commands")]
private void OnGetCandidatesCommand()
{
if (!OnInvokeWithResult(NativeContract.NEO.Hash, "getCandidates", out StackItem result, null, null, false)) return;
if (!OnInvokeWithResult(NativeContract.NEO.Hash, VoteMethods.GetCandidates, out var result, null, null, false)) return;

var resJArray = (Array)result;

Expand All @@ -166,7 +131,7 @@ private void OnGetCandidatesCommand()
[ConsoleCommand("get committee", Category = "Vote Commands")]
private void OnGetCommitteeCommand()
{
if (!OnInvokeWithResult(NativeContract.NEO.Hash, "getCommittee", out StackItem result, null, null, false)) return;
if (!OnInvokeWithResult(NativeContract.NEO.Hash, VoteMethods.GetCommittee, out StackItem result, null, null, false)) return;

var resJArray = (Array)result;

Expand All @@ -188,7 +153,7 @@ private void OnGetCommitteeCommand()
[ConsoleCommand("get next validators", Category = "Vote Commands")]
private void OnGetNextBlockValidatorsCommand()
{
if (!OnInvokeWithResult(NativeContract.NEO.Hash, "getNextBlockValidators", out StackItem result, null, null, false)) return;
if (!OnInvokeWithResult(NativeContract.NEO.Hash, VoteMethods.GetNextBlockValidators, out var result, null, null, false)) return;

var resJArray = (Array)result;

Expand All @@ -210,32 +175,32 @@ private void OnGetNextBlockValidatorsCommand()
[ConsoleCommand("get accountstate", Category = "Vote Commands")]
private void OnGetAccountState(UInt160 address)
{
const string notice = "No vote record!";
const string Notice = "No vote record!";
var arg = new JObject
{
["type"] = "Hash160",
["value"] = address.ToString()
};

if (!OnInvokeWithResult(NativeContract.NEO.Hash, "getAccountState", out var result, null, new JArray(arg))) return;
if (!OnInvokeWithResult(NativeContract.NEO.Hash, VoteMethods.GetAccountState, out var result, null, new JArray(arg))) return;
Console.WriteLine();
if (result.IsNull)
{
ConsoleHelper.Warning(notice);
ConsoleHelper.Warning(Notice);
return;
}
var resJArray = (Array)result;
if (resJArray is null)
{
ConsoleHelper.Warning(notice);
ConsoleHelper.Warning(Notice);
return;
}

foreach (var value in resJArray)
{
if (value.IsNull)
{
ConsoleHelper.Warning(notice);
ConsoleHelper.Warning(Notice);
return;
}
}
Expand All @@ -258,5 +223,28 @@ private void OnGetAccountState(UInt160 address)
ConsoleHelper.Error("Error parsing the result");
}
}
/// <summary>
/// Get account or log a warm
/// </summary>
/// <param name="account"></param>
/// <returns>account or null</returns>
private WalletAccount? GetValidAccountOrWarn(UInt160 account)
{
var acct = CurrentWallet?.GetAccount(account);
if (acct == null)
{
ConsoleHelper.Warning("This address isn't in your wallet!");
return null;
}
if (acct.Lock || acct.WatchOnly)
{
ConsoleHelper.Warning("Locked or WatchOnly address.");
return null;
}
return acct;
}

private byte[] BuildNeoScript(string method, params object?[] args)
=> NativeContract.NEO.Hash.MakeScript(method, args);
Copy link
Member

@cschuchardt88 cschuchardt88 Jun 30, 2025

Choose a reason for hiding this comment

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

I prefer you to use ScriptBuilder class for building scripts. Its easier to understand what is going on. MakeScript is for UInt160 class.

Copy link
Contributor

@Wi1l-B0t Wi1l-B0t Jun 30, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Why add UInt160Extension.MakeScript?
It's useless？

Copy link
Member

@cschuchardt88 cschuchardt88 Jun 30, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Why add UInt160Extension.MakeScript? It's useless？

Yes the name is useless. Name needs to change to something else. That describes what it is really doing.

The method is called BuildNativeScript. Which defeats the purpose of using NativeContract.NEO.Hash. If its only for neo's hash, then the method shouldn't be called BuildNativeScript. Because, it doesn't do what the name suggests.

Copy link
Member

@shargon shargon Jun 30, 2025

Choose a reason for hiding this comment

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

I prefer you to use ScriptBuilder class for building scripts. Its easier to understand what is going on. MakeScript is for UInt160 class.

It works, and it's faster, so it's good

Copy link
Member

@cschuchardt88 cschuchardt88 Jun 30, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

It works, and it's faster, so it's good

Why do you make up things? How is it faster than ScriptBuilder?

image

He needs to rename the method to a meaningful name or change the method code to do that method name implies.

Copy link
Member

@shargon shargon Jun 30, 2025

Choose a reason for hiding this comment

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

Why this is wrong?

Copy link
Member

@shargon shargon Jun 30, 2025

Choose a reason for hiding this comment

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

Yes the name is useless. Name needs to change to something else. That describes what it is really doing.

The method is called BuildNativeScript. Which defeats the purpose of using NativeContract.NEO.Hash. If its only for neo's hash, then the method shouldn't be called BuildNativeScript. Because, it doesn't do what the name suggests.

Focus, if what you're talking about is that you want to rename an extension, you don't have to block a PR, this PR isn't about that.

Copy link
Contributor

@Jim8y Jim8y Jun 30, 2025

Choose a reason for hiding this comment

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

@cschuchardt88 chill, lets focus on functionality and then you can update it at any time if you think its necessary. coding style is a thing we can never reach agreement, people will alawys have different standards.

Copy link
Member

@cschuchardt88 cschuchardt88 Jun 30, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I'm sick of cleaning up everyone's bad habits. What are rules for if no one will follow them. I don't blame @ajara87 because he doesn't know better. I blame the rest of you allowing other new people to pickup on these bad habits and allow them to be merged. If we teach the new people how we want code. I won't have to cleanup 3 other peoples work. I guess now make it four.

https://github.com/neo-project/.github/blob/d83897afb4fd5bd3c107a8dfd845e6dcf1e8d713/docs/neo-coding-rules.md

Focus, if what you're talking about is that you want to rename an extension, you don't have to block a PR, this PR isn't about that.

What are you talking about? I already stated what I want. It has nothing to do with extension methods. It's the name of his method that is being added in this PR. Plus it blocked until @superboyiii tests it anyways. Nothing is a rush or must have merged now.

}
}