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

Add circulating supply endpoint #26

Merged
merged 4 commits into from
Mar 1, 2024
Merged

Conversation

gianfra-t
Copy link
Contributor

@gianfra-t gianfra-t commented Feb 19, 2024

Original task #242

As discussed in the issue, we will define the circulating supply to be equal to the total transferable amount subtracted by the total transferable of the multi-sig accounts.

Implementation

We have already defined the total transferable to be the free - frozen balance. In this PR we just mantain a new variable called supplyToIgnore and during account iteration, we count the transferable amount contribution of the accounts we want to ignore, so the total circulation will end up being totalTransferable - supplyToIgnore.

@gianfra-t gianfra-t requested a review from ebma February 20, 2024 12:55
Copy link

@bogdanS98 bogdanS98 left a comment

Choose a reason for hiding this comment

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

This looks good to me, only left one comment which is just a suggestion 👍🏼

} else {
console.error("Invalid network");
}
// Supply to ignore is the sum of the free - frozen of the accounts to substract

Choose a reason for hiding this comment

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

Can you please rephrase this comment a bit so that's easier to understand? More like explaining why we do this first, then explaining how we do it. It's just a suggestion though, I don't mind leaving it like 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.

Rephrased. Please let me know if it still not so clear.

Choose a reason for hiding this comment

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

That's better, I left one more suggestion.

Copy link
Member

@ebma ebma left a comment

Choose a reason for hiding this comment

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

I think the logic is good overall 👍

src/api/controllers/token.controller.js Outdated Show resolved Hide resolved
src/api/utils/circulating_accounts_to_ignore.js Outdated Show resolved Hide resolved
src/api/utils/circulating_accounts_to_ignore.js Outdated Show resolved Hide resolved
src/api/utils/circulating_accounts_to_ignore.js Outdated Show resolved Hide resolved
src/api/routes/v1/stats.route.js Outdated Show resolved Hide resolved
@gianfra-t gianfra-t merged commit 5d50242 into main Mar 1, 2024
1 check passed
@ebma ebma deleted the circulating-supply-endpoint branch March 1, 2024 09:03
@gianfra-t gianfra-t restored the circulating-supply-endpoint branch April 23, 2024 13:56
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

3 participants