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

feat: save failed submsg result in reply for timelocked contract #ntrn-80 #78

Closed
wants to merge 25 commits into from
Closed

feat: save failed submsg result in reply for timelocked contract #ntrn-80 #78

wants to merge 25 commits into from

Conversation

NeverHappened
Copy link
Contributor

@NeverHappened NeverHappened commented Aug 28, 2023 •
edited
Loading

https://hadronlabs.atlassian.net/browse/NTRN-80

  • in single, multiple and subdao timelock proposals when close_proposal_on_execution_failure = true in config and proposal fails to execution, we save the error to be able to query it (because we can’t see it since reply handles tx error)
  • added query to these modules to look for the error saved
  • to get close_proposal_on_execution_failure in timelock, added code to query for proposal module from subdao contract

@NeverHappened NeverHappened self-assigned this Aug 28, 2023
@NeverHappened NeverHappened force-pushed the feat/save-failed-result branch 2 times, most recently from eca866c to 00c47df Compare August 29, 2023 10:21
@NeverHappened
Copy link
Contributor Author

https://github.com/neutron-org/neutron-tests/actions/runs/6012022326

@NeverHappened NeverHappened force-pushed the feat/save-failed-result branch from 00c47df to 4a733f4 Compare August 29, 2023 12:11
@NeverHappened NeverHappened marked this pull request as ready for review August 29, 2023 13:26
@NeverHappened
Copy link
Contributor Author

https://github.com/neutron-org/neutron-tests/actions/runs/6013284667

@NeverHappened NeverHappened changed the base branch from main to sdk-47 September 28, 2023 08:33
sotnikov-s
sotnikov-s reviewed Sep 28, 2023
View reviewed changes
contracts/dao/proposal/cwd-proposal-multiple/src/testing/tests.rs Outdated
Comment on lines 2210 to 2213
assert_eq!(query_errs.errors.len(), 2);
let error2 = query_errs.errors.last().unwrap();
assert_eq!(error2.error, "error2".to_string());
assert_eq!(error2.height, env2.block.height);
Copy link
Contributor

Choose a reason for hiding this comment

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

let's also check that the first recorded error info persists, and that the status is Status::ExecutionFailed, huh? here and in other test files

contracts/subdaos/cwd-subdao-timelock-single/src/testing/tests.rs Show resolved Hide resolved
@NeverHappened
Copy link
Contributor Author

https://github.com/neutron-org/neutron-tests/actions/runs/6453856477

sotnikov-s
sotnikov-s approved these changes Oct 11, 2023
View reviewed changes
Copy link
Contributor

@sotnikov-s sotnikov-s left a comment

Choose a reason for hiding this comment

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

LGTM but please 1) fix and run integration tests 2) attach a tests PR link to the description of this PR

@NeverHappened
Copy link
Contributor Author

https://github.com/neutron-org/neutron-tests/actions/runs/6481625301

@NeverHappened NeverHappened changed the base branch from sdk-47 to neutron_audit_informal_04_09_2023 October 11, 2023 13:47
@NeverHappened NeverHappened changed the base branch from neutron_audit_informal_04_09_2023 to sdk-47 October 11, 2023 13:48
pr0n00gler added a commit that referenced this pull request Oct 11, 2023
@pr0n00gler
backport of #78
d07d7c5
@pr0n00gler pr0n00gler mentioned this pull request Oct 11, 2023
Merged
@pr0n00gler
Copy link
Collaborator

Merged in #83

@pr0n00gler pr0n00gler closed this Oct 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pr0n00gler pr0n00gler pr0n00gler approved these changes

@sotnikov-s sotnikov-s sotnikov-s approved these changes

@quasisamurai quasisamurai Awaiting requested review from quasisamurai

@swelf19 swelf19 Awaiting requested review from swelf19

Assignees

@NeverHappened NeverHappened

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@NeverHappened @pr0n00gler @sotnikov-s