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.

Sign up for GitHub

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

Jump to bottom

fix/#68-incorrect-authorization-action-of-create-comunity-action #70

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

fix/#68-incorrect-authorization-action-of-create-comunity-action #70

wants to merge 1 commit into from

Conversation

danielvo11
Copy link
Contributor

This merge did:

  • fix authorization actor should be payer in transfer token action

@danielvo11 danielvo11 requested a review from ntpdung June 25, 2020 03:56
@danielvo11
Copy link
Contributor Author

@manh-vv take a look if you have time!

manh-vv
manh-vv requested changes Jun 25, 2020
View reviewed changes
Copy link
Member

@manh-vv manh-vv left a comment

Choose a reason for hiding this comment

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

finished my review

src/CanCommunity.ts
@@ -319,7 +319,7 @@ export class CanCommunity {
actions: [
{
account: 'eosio.token',
authorization: [{ actor: input.creator, permission: 'active' }],
authorization: [{ actor: payer ? payer : input.creator, permission: 'active' }],
Copy link
Member

Choose a reason for hiding this comment

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

We have a discussion in which you told me we need at least two signature.

  • from creator
  • from payer

Is the statement still valid? If so, is this change still valid?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Let me represent the follow that payer want to create community for other one, that include two actions:

  1. payer authorize to transfer CAT token to governance contract account to create community account, so actor here should be payer not creator, that's why I fix this.
  2. creator authorize to update community information with action create in governance contract

Copy link
Contributor

Choose a reason for hiding this comment

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

@danielalvess @manh-vv please help me resolve this MR. Looks like it has been here for a long time !?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@duyphanz duyphanz duyphanz left review comments

@manh-vv manh-vv manh-vv requested changes

@ntpdung ntpdung Awaiting requested review from ntpdung

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@danielvo11 @manh-vv @duyphanz