Skip to content

Tokenomics#7

Closed
bwhm wants to merge 4 commits intoJoystream:masterfrom
bwhm:tokenomics
Closed

Tokenomics#7
bwhm wants to merge 4 commits intoJoystream:masterfrom
bwhm:tokenomics

Conversation

@bwhm
Copy link
Contributor

@bwhm bwhm commented Jul 9, 2020

Made a very rough version in #6 , but will replace with this now.

Not exactly clean code, as I am both not the best coder, and have had to do it in patches. Not sure we can have this in a public repo w/o cleaning it up...?

@bwhm bwhm added the enhancement New feature or request label Jul 9, 2020
@bwhm bwhm requested a review from mnaamani July 9, 2020 13:46
@mnaamani
Copy link
Member

mnaamani commented Jul 10, 2020

Building works, but then I tried to run get-tokenomics-history:

MokoBook:api-examples mokhtar$ node lib/tokenomics/get-tokenomics-history
/Users/mokhtar/joystream/api-examples/lib/tokenomics/get-history-events.js:78
        title: proposalData.get("title")?.toString(),
                                         ^

SyntaxError: Unexpected token '.'
    at wrapSafe (internal/modules/cjs/loader.js:1054:16)
    at Module._compile (internal/modules/cjs/loader.js:1102:27)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1158:10)
    at Module.load (internal/modules/cjs/loader.js:986:32)
    at Function.Module._load (internal/modules/cjs/loader.js:879:14)
    at Module.require (internal/modules/cjs/loader.js:1026:19)
    at require (internal/modules/cjs/helpers.js:72:18)
    at Object.<anonymous> (/Users/mokhtar/joystream/api-examples/lib/tokenomics/get-tokenomics-history.js:6:30)
    at Module._compile (internal/modules/cjs/loader.js:1138:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1158:10)

tsconfig.json might need some tweaking because that ?. syntax is valid in TS but doesn't seem to have been transpiled into valid javascript.

@mnaamani
Copy link
Member

I guess I was using old version of node v12.18.2 so I changed target from esnext to es2019 and running now works.
waiting for program to complete to report my results..

Copy link
Member

@mnaamani mnaamani left a comment

Choose a reason for hiding this comment

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

Works, as long as you connect to an archival node.

Left a few tips and suggestions. If you just need a quick and dirty script, I would say just update the trarget in tsconfig.json to es2019 to make it work with node LTS (v12.x)

}


export async function getPoolStart(api: ApiPromise, burnAddress:string, firstBlock:number, topUps:PoolTopup[], allExchanges: ExchangeInfo[]): Promise<PoolStatus> {
Copy link
Member

Choose a reason for hiding this comment

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

I was gonna suggest a better name would be getPoolStatus()
But I see the other getPoolEnd() which also returns the same type.

After understanding more what you are computing at the start and end, it makes more sense to actually rename PoolStatus to something more appropriate but its difficult to pick a name because it contains both status values and delta values, which suggests you need two separate types, one for each category.

But I think its overkill so carry on :)

feesPaid: feesPaid,
slashAmount: slashAmount,
proposalsAddedInRange: proposalsAddedInRange,
proposalsFinalizedInRange: proposalsFinalizedInRange,
Copy link
Member

Choose a reason for hiding this comment

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

a nice shorthand way of writing this when the key and value names are identical is:

 const proposalData: OverviewProposal = {
    activeProposalsAtStart,
    activeProposalsAtEnd,
    fundingCosts,
    feesPaid,
    slashAmount,
    proposalsAddedInRange,
    proposalsFinalizedInRange,
}



export async function getPoolStart(api: ApiPromise, burnAddress:string, firstBlock:number, topUps:PoolTopup[], allExchanges: ExchangeInfo[]): Promise<PoolStatus> {
let totalAddedToPool = 0
Copy link
Member

Choose a reason for hiding this comment

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

The code is fragile because you are using a plain javascript number for the Balance type which in the runtime is u128.

This is fine as long as the values we are dealing with are not larger than

export async function getGeneralOverview(api: ApiPromise, firstBlockHash:Hash,lastBlockHash:Hash): Promise<OverviewGeneral> {
const forumCategoriesAtStart = await api.query.forum.nextCategoryId.at(firstBlockHash) as CategoryId;
const forumCategoriesAtEnd = await api.query.forum.nextCategoryId.at(lastBlockHash) as CategoryId;
const newCategories = forumCategoriesAtEnd.toNumber()-forumCategoriesAtStart.toNumber()
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
const newCategories = forumCategoriesAtEnd.toNumber()-forumCategoriesAtStart.toNumber()
const newCategories = forumCategoriesAtEnd.sub(forumCategoriesAtStart).toNumber()

Easier on the eyes :)

proposals[index].executedAt = blockHeight
}
} else {
if (decision.type === "Canceled" ||decision.type === "Cancelled") {
Copy link
Member

Choose a reason for hiding this comment

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

why are we checking for two different spellings ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought Canceled was incorrect (one "l"), so I figured it may be changed in the future :)

A quick search uncovered that it's one of the US/British English differences, so it makes less sense!

}
} else {
if (decision.type === "Canceled" ||decision.type === "Cancelled") {
proposals[index].feePaid = 10000
Copy link
Member

Choose a reason for hiding this comment

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

wondering where this number comes from?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't know how to find them with the API, so I used manual inputs. From memory, they're hardcoded in pioneer even.

If I'm wrong, I'd be happy to correct it!

}
// Get tokens burned from tx fees
if (
sectionName === "actors" || sectionName ==="contentWorkingGroup" || sectionName === "councilElection" ||
Copy link
Member

Choose a reason for hiding this comment

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

a pattern like this could work well here:

let sectionNames = ['actors', 'contentWorkingGroup', ....]
if ( sectionNamesToLookFor.contains(sectionName) && otherChecks ...){

}

(sectionName === "members" && methodName !== "buyMembership") ||
(sectionName === "session" && methodName=== "setKeys")
) {
transactionFees ++
Copy link
Member

Choose a reason for hiding this comment

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

assumption that transaction fee is always 1 token is okay, but we will miss this when that changes.

in network integration tests there are some examples on how tx fees are calculated. It could get more complicated in future if weights of transactions also affect the transaction fee (like gas).

To keep it very simple I would have some function at the top function getFeePerTransaction() { return 1 },

and instead of transactionFees++ do transactionFees += getFeePerTransaction() so it will be easy to update later

@bwhm
Copy link
Contributor Author

bwhm commented Sep 26, 2020

Closing, as it's pretty much outdated...

@bwhm bwhm closed this Sep 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants