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

fix(executor): Added register trigger permissions in the default executor #4616

Conversation

Stukalov-A-M
Copy link
Contributor

Description

Linked issue

Benefits

Checklist

  • I've read CONTRIBUTING.md
  • I've used the standard signed-off commit format (or will squash just before merging)
  • All applicable CI checks pass (or I promised to make them pass later)
  • (optional) I've written unit tests for the code changes
  • I replied to all comments after code review, marking all implemented changes with thumbs up

@Stukalov-A-M
Copy link
Contributor Author

@mversic @Erigara
Proposal:
I think we should remove connection of a trigger with a domain_id. It doesn't make sense because each trigger's authority already has this connection. Moreover, it creates some redundant difficulties in cases of permission granting and checking ref is_trigger_owner()

@Stukalov-A-M Stukalov-A-M added blocked this problem can't be fixed yet and removed Bug Something isn't working blocked this problem can't be fixed yet labels May 19, 2024
@Stukalov-A-M Stukalov-A-M changed the title [Fix] #4586: Added register trigger permissions check fix(default executor): Added register trigger permissions check May 20, 2024
@Stukalov-A-M Stukalov-A-M added Bug Something isn't working and removed iroha2 labels May 20, 2024
@Stukalov-A-M Stukalov-A-M changed the title fix(default executor): Added register trigger permissions check fix(executor): Added register trigger permissions check in a default executor May 20, 2024
@Stukalov-A-M Stukalov-A-M changed the title fix(executor): Added register trigger permissions check in a default executor fix(executor): Added register trigger permissions May 20, 2024
@Stukalov-A-M Stukalov-A-M force-pushed the Check-authority-on-trigger-registration branch from e4f8da5 to b4d76d9 Compare May 20, 2024 11:21
@Erigara
Copy link
Contributor

Erigara commented May 20, 2024

I think we should remove connection of a trigger with a domain_id. It doesn't make sense because each trigger's authority already has this connection. Moreover, it creates some redundant difficulties in cases of permission granting and checking ref is_trigger_owner()

Sounds reasonable.

@Stukalov-A-M Stukalov-A-M changed the base branch from iroha2-dev to main May 20, 2024 15:01
@Stukalov-A-M Stukalov-A-M force-pushed the Check-authority-on-trigger-registration branch from b4d76d9 to 855c850 Compare May 20, 2024 15:52
@Stukalov-A-M Stukalov-A-M changed the title fix(executor): Added register trigger permissions fix(executor): Added register trigger permissions in the default executor May 20, 2024
@Stukalov-A-M Stukalov-A-M force-pushed the Check-authority-on-trigger-registration branch 2 times, most recently from 0eb57e6 to 02376ad Compare May 21, 2024 05:42
@Stukalov-A-M
Copy link
Contributor Author

I think we should remove connection of a trigger with a domain_id. It doesn't make sense because each trigger's authority already has this connection. Moreover, it creates some redundant difficulties in cases of permission granting and checking ref is_trigger_owner()

Sounds reasonable.

The ticket is created
#4630 (comment)

@Stukalov-A-M Stukalov-A-M enabled auto-merge (squash) May 21, 2024 07:08
@Stukalov-A-M Stukalov-A-M force-pushed the Check-authority-on-trigger-registration branch from 02376ad to c02144b Compare May 21, 2024 07:13
@Stukalov-A-M Stukalov-A-M enabled auto-merge (squash) May 21, 2024 10:14
@Stukalov-A-M Stukalov-A-M force-pushed the Check-authority-on-trigger-registration branch from c02144b to 29beb5b Compare May 21, 2024 10:15
@Stukalov-A-M Stukalov-A-M merged commit 5310c8b into hyperledger:main May 21, 2024
10 of 11 checks passed
@Stukalov-A-M Stukalov-A-M deleted the Check-authority-on-trigger-registration branch May 21, 2024 13:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working iroha2
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Check authority on trigger registration
5 participants