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

mycelo: Adds missing migrations & a few fixes #1409

Merged
merged 24 commits into from
Mar 3, 2021

Conversation

mcortesi
Copy link
Contributor

@mcortesi mcortesi commented Mar 2, 2021

Description

This PR adds:

  • Attestation contract migration
  • Governance contract migration
  • ElectValidators migration
  • Fix LockedGold & Accounts contract deployment address (was the same)
  • Fix absurdly high initial balance for accounts (surpassed the 1billion CELO limit)
  • Adapt migrations to Contract Release 3

Other changes

  • Adds Token type that pretty prints token balances (18 decimals)
  • Moves genesis commands to their own file.

Tested

Run mycelo multiple times, check status and elections on epoch blocks, rewards distribution, etc.

Related issues

Backwards compatibility

Yes

@mcortesi mcortesi requested a review from a team as a code owner March 2, 2021 20:11
@mcortesi mcortesi requested review from gastonponti, trianglesphere and oneeman and removed request for a team March 2, 2021 20:11
@oneeman
Copy link
Contributor

oneeman commented Mar 2, 2021

I checked that celocli network:contracts works, and it does. But I noticed that the TransferWhitelist contract doesn't have an address listed in core_contracts.go, and is deployed using contract.DeployCoreContract() (so it gets a random-looking address), unlike all the others which are deployed using ctx.deployCoreContract(). Don't know if there is a reason for this or whether we should change it to work like all the others.

Copy link
Contributor

@oneeman oneeman left a comment

Choose a reason for hiding this comment

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

Added some minor comments here and there. Overall looks good.

common/token/token.go Outdated Show resolved Hide resolved
common/token/token.go Outdated Show resolved Hide resolved
)

func TestToString(t *testing.T) {
RegisterTestingT(t)
Copy link
Contributor

Choose a reason for hiding this comment

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

The docs say RegisterTestingT is deprecated and to use NewWithT

Copy link
Contributor

Choose a reason for hiding this comment

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

(This is also used in places not touched by this PR)

common/token/token_test.go Outdated Show resolved Hide resolved
mycelo/env/core_contracts_test.go Show resolved Hide resolved
mycelo/genesis/config.go Outdated Show resolved Hide resolved
mycelo/genesis/genesis_state.go Outdated Show resolved Hide resolved
mycelo/genesis/genesis_state.go Outdated Show resolved Hide resolved
mycelo/genesis/genesis_state.go Outdated Show resolved Hide resolved
mycelo/genesis/genesis_state.go Outdated Show resolved Hide resolved
@oneeman
Copy link
Contributor

oneeman commented Mar 2, 2021

Also, I noticed the failing lint, which is due to spelling. The commission one I noticed myself, but the linter also caught posession.

@mcortesi mcortesi requested a review from oneeman March 3, 2021 01:12
Copy link
Contributor

@oneeman oneeman left a comment

Choose a reason for hiding this comment

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

Great! By the way, I will use this new decimal package to define another type with precision 1 (just a wrapper to serialize and deserialize big.Int as strings in the gencodec overrides where needed)

Copy link
Contributor

@trianglesphere trianglesphere left a comment

Choose a reason for hiding this comment

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

Still a lint, but lgtm

@oneeman
Copy link
Contributor

oneeman commented Mar 3, 2021

I added a commit fixing a bug when generating genesis with only one validator.

@oneeman
Copy link
Contributor

oneeman commented Mar 3, 2021

Added another commit, fixing the developer accounts' cUSD balances which were in Wei rather than cUSD. Also merged master because github said there was a merge conflict (it was due to mycelo commands having moved to a separate file, easy to resolve).

@mcortesi mcortesi merged commit d76deb4 into master Mar 3, 2021
@mcortesi mcortesi deleted the oneeman/mycelo-missing-contracts branch March 3, 2021 22:09
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.

3 participants