-
Notifications
You must be signed in to change notification settings - Fork 27
Security review #253
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
Security review #253
Conversation
✅ Deploy Preview for tansu canceled.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR strengthens the governance contract by implementing multiple security improvements and addressing unsafe patterns. It adds protective checks for contract validation, voting timeline enforcement, and input validation while clarifying governance rules with better documentation.
Key changes include:
- Enhanced input validation for voting operations and upgrade proposals
- Strict voting deadline enforcement to prevent late votes
- Improved error handling with safer unwrap alternatives
- Documentation updates explaining supermajority governance model
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
website/docs/developers/governance.mdx | Added comprehensive documentation for voting timeline, supermajority rules, and eligibility requirements |
contracts/tansu/src/tests/test_pause_upgrade.rs | Added test coverage for invalid upgrade configuration scenarios |
contracts/tansu/src/tests/test_dao.rs | Commented out flaky event verification test |
contracts/tansu/src/tests/test_commit.rs | Added tests for anonymous vote commitment validation edge cases |
contracts/tansu/src/lib.rs | Improved contract validation logic structure |
contracts/tansu/src/contract_tansu.rs | Added admin threshold validation and safer error handling |
contracts/tansu/src/contract_membership.rs | Improved member project badge lookup logic |
contracts/tansu/src/contract_dao.rs | Enhanced voting validation, deadline checks, and anonymous vote processing |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
✅ Deploy Preview for staging-tansu ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
9f74935
to
145cf99
Compare
@tupui PR open |
Thanks I will have a look latest tomorrow morning |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this, I just have one comment and I think this is good to go otherwise.
contracts/tansu/src/contract_dao.rs
Outdated
// Use get() method to access elements safely | ||
let voted_approve = match tallies.get(0) { | ||
Some(v) => v, | ||
None => return types::ProposalStatus::Cancelled, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the nitpick but we actually don't want to mark it as cancelled if there is an issue. Otherwise that's an easy way for people to mark proposals as cancelled which is a real outcome.
Instead we want to fail. I probably need to add some tools to modify some bad votes or something like this but first I want to see if that's really needed and how people try to abuse the system.
I am also not sure if that can even happen if we get to that point in the code.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
145cf99
to
64b677a
Compare
64b677a
to
2107eaf
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks again!
Strengthens the governance contract by addressing unsafe patterns, clarifying rules, and adding protective checks. It improves security, maintainability, and auditability.
CLeanup from #239