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

Refactor peg token redeemption #367

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Conversation

Gerard097
Copy link
Collaborator

@Gerard097 Gerard097 commented Nov 12, 2023

  • Removed dynamic edge name generation for Fast DAO id lookup in redemption process (Replaced with token to DAO table)
  • Modified assigntokdao to specify which token is assigned (peg | reward)

@Gerard097 Gerard097 requested a review from n13 November 12, 2023 23:52
src/dao.cpp Outdated
@@ -3517,6 +3561,7 @@ void dao::reset() {
delete_table<election_vote_table>(get_self(), 2);
delete_table<election_vote_table>(get_self(), 3);
delete_table<token_to_dao_table>(get_self(), get_self().value);
Copy link
Collaborator

Choose a reason for hiding this comment

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

should this be reward_token_to_dao_table instead of the old "token_to_dao_table"?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I mean it is a alias for that, but for enhanced reading I can use that instead. Pushed to the next commit

@n13
Copy link
Collaborator

n13 commented Nov 13, 2023

Made one comment.

Also kinda wondering why tokens aren't in a document, so there could be an edge dao_id -> TOKEN_EDGE_NAME -> TokenDoc (pegToken, rewardToken) ... then we would not need an extra table?

Maybe too much for this PR but wondering why token info is in a separate table.

Copy link
Collaborator

@n13 n13 left a comment

Choose a reason for hiding this comment

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

Left one comment please check

@Gerard097
Copy link
Collaborator Author

Gerard097 commented Nov 23, 2023

Made one comment.

Also kinda wondering why tokens aren't in a document, so there could be an edge dao_id -> TOKEN_EDGE_NAME -> TokenDoc (pegToken, rewardToken) ... then we would not need an extra table?

Maybe too much for this PR but wondering why token info is in a separate table.

In a sense, we already do this as we store the tokens in the DAO's settings document i.e. dao_id -> settings -> SettingsDoc (reward_token, peg_token). The actual issue is that for transfer action we don't know the DAO ID in advance, and we need to do the lookup by the Token symbol. We could iterate over all the settings documents and search for the one that matches the token but that would be very inefficient and could lead to more timeouts

@n13

@n13
Copy link
Collaborator

n13 commented Nov 24, 2023

I see yeah because we don't get any info from the transfer listener other than the symbol... so we need to look up the symbol either way...

Copy link
Collaborator

@n13 n13 left a comment

Choose a reason for hiding this comment

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

LGTM

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