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

Community address #1498

Merged
merged 7 commits into from
May 1, 2020
Merged

Community address #1498

merged 7 commits into from
May 1, 2020

Conversation

AdoAdoAdo
Copy link
Contributor

@AdoAdoAdo AdoAdoAdo commented Apr 30, 2020

Added rewards for community
At the end of the epoch a percentage defined in config from the protocol rewards will be directed towards a community account

@raduchis raduchis self-requested a review April 30, 2020 18:45
@sasurobert sasurobert self-requested a review April 30, 2020 19:17
uint64 PrevEpochStartRound = 6;
bytes PrevEpochStartHash = 7;
bytes RewardsForCommunity = 5 [(gogoproto.casttypewith) = "math/big.Int;github.com/ElrondNetwork/elrond-go/data.BigIntCaster"];
bytes CommunityAddress = 6;
Copy link
Contributor

Choose a reason for hiding this comment

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

There is no need for this to appear in the metablock. It is enough to appear in the generated reward transactions

Copy link
Contributor

Choose a reason for hiding this comment

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

the bigInt of rewards for community can remain as it is super visible, but the address can be read from the config.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

func (e *economics) computeRewardsForCommunity(totalRewards *big.Int) *big.Int {
precentageAdjustment := float64(100)
communityPercentage := big.NewInt(0).SetUint64(uint64(e.rewardsHandler.CommunityPercentage() * precentageAdjustment))
rewardsForCommunity := big.NewInt(0).Mul(totalRewards, communityPercentage)
Copy link
Contributor

Choose a reason for hiding this comment

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

you can use core.GetPercentageOfValue instead of this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

metaBlock *block.MetaBlock,
) (*rewardTx.RewardTx, []byte, uint32, error) {

communityAddress := metaBlock.EpochStart.Economics.CommunityAddress
Copy link
Contributor

Choose a reason for hiding this comment

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

community address may be saved as an internal variable to the rewardsCreator - and this check can be done, address validity at the time of constructor.

config/tomlConfig_test.go Outdated Show resolved Hide resolved
@@ -138,6 +141,7 @@ func TestTomlEconomicsParser(t *testing.T) {
[RewardsSettings]
RewardsValue = "` + rewardsValue + `"
Copy link
Contributor

Choose a reason for hiding this comment

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

There is no rewardsValue and burnPercentage any more.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@@ -179,11 +183,16 @@ func convertValues(economics *config.EconomicsConfig) (*EconomicsData, error) {
func checkValues(economics *config.EconomicsConfig) error {
if isPercentageInvalid(economics.RewardsSettings.LeaderPercentage) ||
Copy link
Contributor

Choose a reason for hiding this comment

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

are these tests enough for the rewards percentages? Could it be that all are below 100% but together account for more than 100%? e.g. LeaderPercentage 80% and DeveloperPercentage 80% in total we have 160% of the fees in a SC call?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

indeed these are not enough.
But there are different criterias as the leader percentage is applied to the accumulated fees in the proposed block, whereas the developerPercentage is applied to the transaction fee calling the speciffic SC, so also not part of the total. The community percentage is applied to the total.

Due to your comment I checked a bit and found a bug in the accumulated fees and what the Leader receives. The accumulated fees are considered entirely when computing the rewards per block, but then again the leader receives in addition the leader percentage -> leader percentage should have been subtracted before computing the rewards per block, otherwise we are giving that portion twice.

Copy link
Contributor

Choose a reason for hiding this comment

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

great find 👍

epochStart/metachain/economics_test.go Show resolved Hide resolved
Copy link
Contributor

@LucianMincu LucianMincu left a comment

Choose a reason for hiding this comment

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

System tests passed.

@LucianMincu LucianMincu merged commit 5898c21 into development May 1, 2020
@LucianMincu LucianMincu deleted the community-address branch May 1, 2020 17:14
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.

None yet

4 participants