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

Preliminary BurgerGov for governance token #3

Open
wants to merge 34 commits into
base: main
Choose a base branch
from

Conversation

Hecate2
Copy link
Contributor

@Hecate2 Hecate2 commented Nov 4, 2021

No description provided.

BurgerGov.cs Outdated Show resolved Hide resolved
BurgerGov.cs Outdated
// DO NOT USE the prefix 0x14, because this is the key prefix for NEP-17 token account balance
[InitialValue("Nb2CHYY5wTh2ac58mTue5S3wpG6bQv5hSY", ContractParameterType.Hash160)]
private static readonly UInt160 DEFAULT_OWNER = default;
private static readonly byte PREFIX_OWNER = 0xff;
Copy link
Contributor

Choose a reason for hiding this comment

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

then PREFIX_OWNER is not necessary

BurgerGov.cs Outdated
[ManifestExtra("Description", "NeoBurger Governance Token")]
[SupportedStandards("NEP-17")]
[ContractPermission("*", "*")]
public class BurgerGov : Nep17Token
Copy link
Contributor

Choose a reason for hiding this comment

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

the name can be NeoBurgerGovernanceToken and the filename can be NeoBurgerGovernanceToken.cs

BurgerGov.cs Outdated
private static readonly byte PREFIX_VOTE = 0xc1;

public override byte Decimals() => 8;
public override string Symbol() => "bNEOg";
Copy link
Contributor

Choose a reason for hiding this comment

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

the symbol can be NOBUG ("NEOBURGER".remove("er"))

BurgerGov.cs Outdated

public override byte Decimals() => 8;
public override string Symbol() => "bNEOg";
public static UInt160 Owner() => (UInt160)Storage.Get(Storage.CurrentContext, new byte[] { PREFIX_OWNER });
Copy link
Contributor

Choose a reason for hiding this comment

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

then Owner is not necessary

BurgerGov.cs Outdated

public static void _deploy(object data, bool update)
{
if (!update)
Copy link
Contributor

Choose a reason for hiding this comment

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

no need to add if (!update) imo

BurgerGov.cs Outdated

public static void Update(ByteString nefFile, string manifest)
{
ExecutionEngine.Assert(Runtime.CheckWitness(Owner()));
Copy link
Contributor

Choose a reason for hiding this comment

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

check witness of the dao itself

BurgerGov.cs Outdated

public static void OnNEP17Payment(UInt160 from, BigInteger amount, object data)
{
throw new System.Exception("Not implemented for now");
Copy link
Contributor

Choose a reason for hiding this comment

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

remove this line to accept airdrop

@vang1ong7ang
Copy link
Contributor

readonly method can be placed above like below in BurgerNEO.cs

public static UInt160 Strategist() => (UInt160)Storage.Get(Storage.CurrentContext, new byte[] { PREFIXSTRATEGIST });

@Hecate2
Copy link
Contributor Author

Hecate2 commented Nov 4, 2021

Then we need another method in BurgerNeoGovernanceToken to check the voting result of proposals and execute the proposal?
Otherwise nobody can call the methods in BurgerNeoGovernanceToken

@vang1ong7ang
Copy link
Contributor

Then we need another method in BurgerNeoGovernanceToken to check the voting result of proposals and execute the proposal

i think so

[ContractPermission("*", "*")]
public class NeoBurgerGovernanceToken : Nep17Token
{
// DO NOT USE the prefix 0x14, because this is the key prefix for NEP-17 token account balance
Copy link
Contributor

Choose a reason for hiding this comment

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

imo all the comments can be removed

@Hecate2
Copy link
Contributor Author

Hecate2 commented Nov 5, 2021

The method CountVote has been preliminarily tested, where the iterator works well. ExecuteProposal can reuse CountVote, but may cost more GAS.

@Hecate2
Copy link
Contributor Author

Hecate2 commented Nov 5, 2021

Ready for further complete tests

@Hecate2
Copy link
Contributor Author

Hecate2 commented Nov 8, 2021

Stored proposal attributes as structs in commit f17807c. Unit-tested.

private static readonly byte PREFIX_DEFAULT_DELEGATE = 0x82;
private static readonly byte PREFIX_DELEGATE_THRESHOLD = 0x83;
private static readonly byte PREFIX_VOTE = 0xc1;
private static readonly byte PREFIX_VOTING_PERIOD = 0xc2;
Copy link
Contributor

Choose a reason for hiding this comment

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

we can mark this one constant and upgrade the contract if needed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants