celo-org / celo-monorepo

Official repository for core projects comprising the Celo platform
https://celo.org
Apache License 2.0
699 stars 369 forks source link

[Bounty] CeloCLI Staking Rewards should show whole Wei #6798

Closed YazzyYaz closed 3 years ago

YazzyYaz commented 3 years ago

[Bounty] CeloCLI Staking Rewards Command should show whole Wei

Challenge Description

This bounty is a bug fix in our celocli command line tool. This bounty allows developers to contribute to core tooling used by many Celo community members.

Running the command:

celocli rewards:show --voter <ADDRESS> --estimate

Shows the output in Address payment in fractional wei (10^18) when it should show whole wei.

Actual Output: 123456789012345678.123456789

Expected Output: 123456789012345678 (value is truncated at decimal point)

Time expectation

Expect this bounty to take a developer familiar with Javascript and about 30 minutes.

Submission Requirements

You will need to submit a PR to the celo-monorepo with the fix and test. Please link this PR in your GitCoin submission. Pay out will be granted on successful merge into the celo-monorepo.

Make sure to submit a Celo address (not an Ethereum address) on the bounty submission page. As Celo is a mobile-first blockchain, we suggest you install Valora to create a new Celo account, but you can also create an account using the command line tool or via a Ledger hardware wallet.

We will be checking your GitHub account activity to verify that you are indeed a unique person and to ensure that the same user is not making multiple submissions. Your request will not be accepted if you do not have an active history on GitHub.

Be aware that it may take two weeks to evaluate and payout bounty submissions.

Resources

What's next?

gitcoinbot commented 3 years ago

Issue Status: 1. Open 2. Started 3. Submitted 4. Done


This issue now has a funding of 8.0 CELO (25.57 USD @ $3.2/CELO) attached to it.

sebasaenz commented 3 years ago

Hi @aslawson, I've being checking up the codebase as well as trying to setup the dev environment. I'm quite sure of how to tackle the issue with the decimals in the rewards output but I'm not being able to get the project running in order to test it.

I've tried building the cli with yarn build from inside the cli folder but I get the error mentioned later (Cannot find module '@celo/contractkit') as well as other modules not being found.

So then I tried building the whole celo-monorepo with the commands bellow but after entering yarn build in the celo-monorepo main folder I get the following error.

lerna notice cli v3.16.0
lerna info versioning independent
lerna info Executing command in 22 packages: "yarn run build"
lerna info run Ran npm script 'build' in '@celo/moonpay-auth' in 1.1s:
$ tsc -b .
lerna info run Ran npm script 'build' in '@celo/attestation-proxy' in 1.3s:
$ tsc -b .
lerna ERR! yarn run build exited 1 in 'docs'
lerna ERR! yarn run build stdout:
$ gitbook install && gitbook build
Error loading version latest: Error: Cannot find module 'is'
    at Function.Module._resolveFilename (internal/modules/cjs/loader.js:636:15)
    at Function.Module._load (internal/modules/cjs/loader.js:562:25)
    at Module.require (internal/modules/cjs/loader.js:692:17)
    at require (internal/modules/cjs/helpers.js:25:18)
    at Object.<anonymous> (/Users/mac/.gitbook/versions/3.2.3/lib/modifiers/summary/insertArticle.js:1:10)
    at Module._compile (internal/modules/cjs/loader.js:778:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:789:10)
    at Module.load (internal/modules/cjs/loader.js:653:32)
    at tryModuleLoad (internal/modules/cjs/loader.js:593:12)
    at Function.Module._load (internal/modules/cjs/loader.js:585:3)

TypeError: Cannot read property 'commands' of null
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

lerna ERR! yarn run build stderr:
error Command failed with exit code 1.

Do you know what could be causing this? I know the whole fix was intended to be solved in around 30 minutes but honestly the setup is getting a little bit complicated.

Thanks!

deepakhb2 commented 3 years ago

Hi @sebasaenz,

I did spend time to setup my local environment and to understand the issue. I think you need to run the command 'yarn build --ignore docs' instead of 'yarn build' will solve the problem. I hope this helps. I think there is some problem while installing the docs and it is not required to be installed.

Regards, Deepak

deepakhb2 commented 3 years ago

I am facing a different problem after connecting to alfajores-forno node with test address. `

./bin/run rewards:show --voter=0xc1912fEE45d61C87Cc5EA59DaE31190FFFFf232d --estimate Running Checks: ✘ 0xc1912fEE45d61C87Cc5EA59DaE31190FFFFf232d is a registered Account 0xc1912fEE45d61C87Cc5EA59DaE31190FFFFf232d is not registered as an account. Try running account:register › Error: Some checks didn't pass! `

I am not sure if I missed something. Also, I saw that bounty is approved to @sebasaenz I wanted to add my comments to understand the problem. Please let me know the problem. Thank you.

aslawson commented 3 years ago

@deepakhb2 thanks for taking interest in Celo! So voters need to be registered accounts on the blockchain themselves. you can solve this by simply running the command that it specifies ./bin/run account:register

aslawson commented 3 years ago

That brings up a good point that to test this you likely need more resources on the voting process. Specifying bounties on gitcoin is new and a learning process for us :)

Celo Owners Guide with Steps to Vote for Validators Locking Gold command for non-releasegold use cases -- since the above apparently expects user to have a releasegold contract General PoS Documentation if interested

barbaraliau commented 3 years ago

@deepakhb2 thanks for jumping in! We will be posting additional bounties so keep an eye out for those 👀

@sebasaenz This is documented in the SETUP.md but that file is understandably difficult to find if you don’t know to look for it. We’ll include a link to it for future bounties. https://github.com/celo-org/celo-monorepo/blob/master/SETUP.md

deepakhb2 commented 3 years ago

@aslawson Thank you, I did try this command ./bin/run account:register. But, it required --from address i.e already an account to register my test address. As I am using https://alfajores-forno.celo-testnet.org node I don't have any address to pass form --from

to register.

sebasaenz commented 3 years ago

@barbaraliau Thanks for the info. I was still having troubles with the setup so I removed all the files and started from scratch. I managed to make the node running, installed all the dependencies and built the cli (and the monorepo) but now I'm getting a segmentation fault error when running ./bin/run rewards:show --voter=<SOME-ADDRESS> --estimate:

[1]    95129 segmentation fault  ./bin/run rewards:show --voter=<SOME-ADDRESS>

After running celocli node:synced I get some info about the last block but after that it returns false. Is there any workaround for avoiding this segmentation fault error?

aslawson commented 3 years ago

you can also point your node to our forno endpoint (we currently run nodes with an api gateway for development though i'm not sure if that's not included in the SETUP.md...) --node https://alfajores-forno.celo-testnet.org

https://docs.celo.org/developer-guide/forno

@deepakhb2 you can make a new account by running ./bin/run account:new

sebasaenz commented 3 years ago

Hi @aslawson, I keep having the same issue. I added the node flag to the docker command but keep getting the same segmentation fault error when running any command. I would really like to get this to work but I don't want to slow down the dev process, even more if @deepakhb2 already started to do some work. I think I'll make a last try after your response and if it doesn't work I'll stop working on the bounty so other people can work on it.

So only to be sure, the docker command should look something like this?

docker run --name celo-fullnode -d --restart unless-stopped -p 127.0.0.1:8545:8545 -p 127.0.0.1:8546:8546 -p 30303:30303 -p 30303:30303/udp -v $PWD:/root/.celo $CELO_IMAGE --verbosity 3 --syncmode full --rpc --rpcaddr 0.0.0.0 --rpcapi eth,net,web3,debug,admin,personal --light.serve 90 --light.maxpeers 1000 --maxpeers 1100 --etherbase $CELO_ACCOUNT_ADDRESS --nousb --alfajores --datadir /root/.celo --node https://alfajores-forno.celo-testnet.org

It already has an --alfajores flag, so I don't know if the node flag you were talking about is for this command. I also tried the flag on ./bin/run account:register but I also get a segmentation fault error. Let me know if there's something else to try. Thanks

deepakhb2 commented 3 years ago

I have currently stopped working on this @sebasaenz.

Even I got stuck in the after ./bin/run account:new creates new account but when I run ./bin/run account:list the created account is not listing as well as account:register is failing with error. > ./bin/run account:register --from 0xC6Cf251f0F8C69a8E91DD9fF0E413df521497311 --name test-account Running Checks: ✔ 0xC6Cf251f0F8C69a8E91DD9fF0E413df521497311 is not a registered Account Sending Transaction: register... failed: unknown account Error: unknown account

@aslawson let me know if I am missing something here. Thank you.

aslawson commented 3 years ago

Love the perseverance :)

@sebasaenz -- sorry, I should have been more clear. the --node command would get added to the celocli call itself

./bin/run rewards:show --voter=<SOME-ADDRESS> --node https://alfajores-forno.celo-testnet.org

Alternatively we have a few other solutions if you really want to run a node locally:

  1. Use our ultralight node. The fullnode you are running will take a long time to get synced (several hours). But we have a lighter version that relies on trusting nearby fullnodes. My guess is that its just not synced yet when you try to run the command 😬 https://docs.celo.org/developer-guide/start/hellocontracts#start-the-node ^ Here is the only place in the docs we mention how to run the light version. Do you mind me asking, what resource directed you to running this full node (so we know where we should be updating docs)?
  2. We just released 1-click nodes as AMIs yesterday. I think only the AWS ones are available.

@deepakhb2's i just ran these steps and there are two things you may be running into:

Hope that helps.

sebasaenz commented 3 years ago

I keep having this segmentation fault error :/ It must be due to some RAM or hard disk requirements my computer is probably not meeting. Anyways, right now I'm working on another big project so I'll be leaving the bounty so someone else can give it a try. Thanks for the help!

gitcoinbot commented 3 years ago

Issue Status: 1. Open 2. Started 3. Submitted 4. Done


Work has been started.

These users each claimed they can complete the work by 265 years, 8 months from now. Please review their action plans below:

1) erazhu31 has been approved to start work.

Hello i would want to try . i have a light node working in the reward folder show file there is toFixed function being used may this can be updated to return decimal 2) leetdev has applied to start work _(Funders only: approve worker | reject worker)_.

I will gladly take a look at the issue and provide a fix. I have completed a couple of introductory bounties to Celo so am somewhat familiar with it, as well as gladly willing to learn more. 3) deepakhb2 has applied to start work _(Funders only: approve worker | reject worker)_.

Hello, I have already started to check the problem and tried setting up the environment. I will continue from there to fix the rewards show command with corresponding tests. Thank you. 4) thaingominhviettel has applied to start work _(Funders only: approve worker | reject worker)_.

Provide an action plan and any initial questions you have for this ticket?what 5) qasimmehdi has applied to start work _(Funders only: approve worker | reject worker)_.

I have good knowledge of javascript I can solve this bug. 6) powvt has applied to start work _(Funders only: approve worker | reject worker)_.

Hello, I can work on this right away. Looks like a quick issue to solve!

Learn more on the Gitcoin Issue Details page.

deepakhb2 commented 3 years ago

Thank you @aslawson. The commands work now. @aslawson How can I get test rewards for my address to validate the change? It will great if you point me to any document that helps to get rewards and verify the command. Thank you.

aslawson commented 3 years ago

@deepakhb2 you will just have to wait the length of the epoch. on mainnet that is 1 day (I believe the same for testnets)

erazhu31 commented 3 years ago

hi @aslawson i used celo-ultralight-node how can reduce the reward calculation time i try by using --epochs=1 it runs then i have Invalid JSON RPC response: "" thanks

aslawson commented 3 years ago

@erazhu31 can you point me to where you identified you can use a --epochs param? i was not aware of that param and i'm guessing it wouldn't be able to reduce the epoch time if the node is running on an existing testnet (only if you were running a local dev blockchain)

erazhu31 commented 3 years ago

sure with this cmd rewards:show --epochs=epochs [default: 1] Show results for the last N epochs thanks

erazhu31 commented 3 years ago

Hi @aslawson i would need some returns on how in import some data to test on the ultra node for the voters reward. so far this it is what i have : the election list and also voters Address Name
0x5edfCe0bad47e24E30625c275457F5b4Bb619241 cLabs(1)
0x87614eD7AF361a563C6a3624CcadD52e165f67C2 cLabs(2)

but there is no reward for them Error: no suitable peers available

for the Validators i have this result:

Validator rewards: Validator Validatorpayment Validatorscore
0x30D060F129817c4DE5fBc1366d53e19f43c8c64f 298763119040969295755 0.9842842575298
0x30D060F129817c4DE5fBc1366d53e19f43c8c64f 299315432138910850400 0.9842842575298

like you can see the validator return is correct.

thanks

aslawson commented 3 years ago

Gotcha -- yes, that will show the rewards over the past N epochs. But it does not speed up the cadence of the epochs (which is one day). If you check now, do your voting accounts have rewards @erazhu31?

gitcoinbot commented 3 years ago

@erazhu31 Hello from Gitcoin Core - are you still working on this issue? Please submit a WIP PR or comment back within the next 3 days or you will be removed from this ticket and it will be returned to an ‘Open’ status. Please let us know if you have questions!

Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days

erazhu31 commented 3 years ago

hi @aslawson now i have also a fullnode up and now it is all synced. regarding the full node when i first query for the rewards:show and i have everything in the answer : groups validators and voters but then i cannot query any more i have this msg from web3-core-helpers/lib/error.js: not an account ->line 74 i am not sure why this appear

For Ultra node It seems there is no any rewards i try and run this : ./run rewards:show --voter 0x5edfCe0bad47e24E30625c275457F5b4Bb619241 Answer:* UnhandledPromiseRejectionWarning: Error: no suitable peers available

thanks

aslawson commented 3 years ago

@erazhu31 if you are using a locally running fullnode, you will either need to unlock the private key on the node or send the privateKey with the celocli command.

if you are running using the docker image, you should create the account following these instructions and use that one as the voter.

otherwise, if you create an account using the celocli account:new you can just pass in the --privateKey as part of your `./run rewards:show --voter 0x5edfCe0bad47e24E30625c275457F5b4Bb619241 --privateKey for development use cases

erazhu31 commented 3 years ago

Hi @aslawson i run this cmd: ./run rewards:show --voter 0xc64DF5Be250264bf2888591D87cdeB13BFADC501 --privateKey 7a0970e7181b9003581c8870a7631addc6a7001c4ae896cd1d0363e528cfaa0b --epochs=20 and i have this msg: "Error: invalid BigNumber string (argument="value", value="c64df5be250264bf2888591d87cdeb13bfadc501", code=INVALID_ARGUMENT, version=bignumber/5.0.13)"

i think in the election ABI the method election.getVoterShare(address) is missing thanks

erazhu31 commented 3 years ago

Hi @aslawson can i have a discord contact regarding election.getVoterShare(address) function i will need more info on the process thanks

aslawson commented 3 years ago

Oh interesting. Looks like someone noticed it and pushed a fix a few days ago. https://github.com/celo-org/celo-monorepo/commit/78899aa21ca7bd55593a4b26fdb46d11c3f97c71 @erazhu31 can you try and git pull and rebuild?

gitcoinbot commented 3 years ago

@erazhu31 Hello from Gitcoin Core - are you still working on this issue? Please submit a WIP PR or comment back within the next 3 days or you will be removed from this ticket and it will be returned to an ‘Open’ status. Please let us know if you have questions!

Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days

erazhu31 commented 3 years ago

Hi @aslawson thanks i run with the updated files and now i have this with toFixed(0): ./run rewards:show --voter 0xc64DF5Be250264bf2888591D87cdeB13BFADC501 --privateKey 7a0970e7181b9003581c8870a7631addc6a7001c4ae896cd1d0363e528cfaa0b --epochs=1 --estimate --csv

Voter rewards: Address,Addresspayment,Group,Averagevalidatorscore,Epochnumber 0xc64DF5Be250264bf2888591D87cdeB13BFADC501,1509126853724092796,0xc64DF5Be250264bf2888591D87cdeB13BFADC501,1,326

i did a PR : #7437 opened 5 minutes ago by erazhu31 • Review required thanks

gitcoinbot commented 3 years ago

@erazhu31 Hello from Gitcoin Core - are you still working on this issue? Please submit a WIP PR or comment back within the next 3 days or you will be removed from this ticket and it will be returned to an ‘Open’ status. Please let us know if you have questions!

Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days

gitcoinbot commented 3 years ago

@erazhu31 Hello from Gitcoin Core - are you still working on this issue? Please submit a WIP PR or comment back within the next 3 days or you will be removed from this ticket and it will be returned to an ‘Open’ status. Please let us know if you have questions!

Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days

erazhu31 commented 3 years ago

hi i still working on it thanks

gitcoinbot commented 3 years ago

Issue Status: 1. Open 2. Started 3. Submitted 4. Done


Work for 8.0 CELO (30.24 USD @ $3.88/CELO) has been submitted by:


erazhu31 commented 3 years ago

Hi @aslawson I used a wrong adr in the submit let me give you a Celo valid one

Valid Celo Adr: 0x7dCf485649BDaCa3EA197F0124b54292F1951dcA

Thanks

aslawson commented 3 years ago

sorry for the delay @erazhu31 -- I am trying to figure out the proper protocol for paying out the to the celo address instead of the one submitted on gitcoin

erazhu31 commented 3 years ago

Hi @aslawson i do not know if you could help me with the issue that i have. In fact i did another Bounty this one : Learn To Send CELO And CUSD and i also used the wrong adr. payment had been done and the 1 Celo on this adr come from this first Bounty but i have not access to this adr Thanks

aslawson commented 3 years ago

@erazhu31 -- if you already were paid to the eth address, you can recover the funds by following the docs: https://docs.celo.org/celo-owner-guide/eth-recovery

otherwise, i would just delete your previous submission and submit a new one with the right address

erazhu31 commented 3 years ago

Hi @aslawson Let us move forward with your second proposition, since I do not have any access on the other adr. I will submit with the correct adr as you mentioned in the new PR, please let me know when to proceed. Thanks

aslawson commented 3 years ago

@erazhu31 that should be sufficient as long as you submit everything required in the task. someone else is reviewing and paying out the funds for that bounty. but if you get rejected, feel free to reach out to me on discord https://discord.com/invite/6yWMkgM

erazhu31 commented 3 years ago

Hi @aslawson Well received Thanks