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

feat: add and remove schedule msgs can be modified in chain manager #ntrn-387 #113

Merged
merged 2 commits into from
Sep 17, 2024

Conversation

NeverHappened
Copy link
Contributor

@NeverHappened NeverHappened commented Aug 27, 2024

@NeverHappened NeverHappened force-pushed the feat/chain-manager-cron-stargate branch from 94286af to a6a80d4 Compare September 3, 2024 14:44
@pr0n00gler
Copy link
Collaborator

Copy link
Contributor

@swelf19 swelf19 left a comment

Choose a reason for hiding this comment

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

LGTM.

} else if typed_proposal.type_field.as_str() == MSG_TYPE_REMOVE_SCHEDULE {
if strategy.has_cron_remove_schedule_permission() {
Ok(())
} else {
Copy link
Contributor

Choose a reason for hiding this comment

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

LGTM. But i think it's possible to save a couple of lines of code. If you remove all 3

} else {
            Err(ContractError::Unauthorized {})
        }

and just return the error Err(ContractError::Unauthorized {}) at the very end if non of if matches

@pr0n00gler pr0n00gler merged commit ccc6b9a into main Sep 17, 2024
13 checks passed
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.

3 participants