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

Remove separate carbonOffsetPartner tracking #207

Merged
merged 1 commit into from
Sep 26, 2023
Merged

Conversation

palango
Copy link
Contributor

@palango palango commented Sep 6, 2023

It seems that CarbonOffsetPartnerStartOf is not actively used.

I'm a bit surprised by that and want to run more testing on this.

  • Investigate epoch rewards calculation in respect to LockedGold and sub-accounts

@palango palango changed the title Remove carbon offset tracking WIP: Remove carbon offset tracking Sep 6, 2023
@eelanagaraj
Copy link
Contributor

Notes about removing this

After looking into this, we confirmed that these rewards are already covered by Rosetta (i.e. doesn’t look like we need to fix a bug/exclude addresses from reconciliation), therefore it's fine to remove the (unused) carbonOffset stuff in Rosetta without needing to add any additional logic to cover epoch rewards calculations.

Details:

  • carbonOffsetPartner is still relevant post-Gingerbread — this is the portion of epoch rewards that is used for offsetting
    • according to CIP-52, eventually this will be redirected to the Green Fund (but simply by setting the carbonOffsettingPartner in EpochRewards.sol to point to this)
  • Ultragreen adds offsetting of a portion of the base fee (20% goes to the new Green Fund, 80% burned)
    blockchain code (in distributeEpochRewards) uses the Mint function (in GoldToken.sol) which emits a Transfer event
    • Rosetta simply filters on all transfers made in the last tx of the block (i.e. epoch reward tx), which should thus already include the carbon offsetting portion of epoch rewards

Tested

Ran reconciliation checks locally on mainnet past epoch 10 (contains epoch rewards paid out to the carbon offsetting partner 0x0ba9f5b3cdd349ab65a8dacda9a38bc525c2e6d6) + marking that account as an interesting account to ensure the balances were reconciled.

@eelanagaraj eelanagaraj marked this pull request as ready for review September 26, 2023 09:36
@eelanagaraj eelanagaraj changed the title WIP: Remove carbon offset tracking Remove separate carbonOffsetPartner tracking Sep 26, 2023
Copy link
Contributor

@eelanagaraj eelanagaraj left a comment

Choose a reason for hiding this comment

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

rebased master, confirmed that this logic is already covered

@eelanagaraj eelanagaraj merged commit 0e5e996 into master Sep 26, 2023
4 checks passed
@eelanagaraj eelanagaraj deleted the palango/cleanup branch September 26, 2023 09:39
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