Skip to content

fix: transferring ownership of pre-existing deployemnts#1177

Open
themariofrancia wants to merge 2 commits into
developmentfrom
fix/find-013
Open

fix: transferring ownership of pre-existing deployemnts#1177
themariofrancia wants to merge 2 commits into
developmentfrom
fix/find-013

Conversation

@themariofrancia
Copy link
Copy Markdown
Contributor

Description

Type of change

  • Bug fix 🐞
  • New feature ✨
  • Breaking change 💥
  • Documentation update 📖
  • Refactor 🔧

Testing

Node version:

  • 20
  • 22
  • 24

Checklist

  • Style Guidelines followed ✅
  • Documentation Updated 📚
  • Linters - No New Warnings ⚠️
  • Local Tests Pass ✅
  • Effective Tests Added ✔️
  • No reduction of Coverage

Signed-off-by: Mario Francia <mariofranciarius@gmail.com>
Signed-off-by: Mario Francia <mariofranciarius@gmail.com>
@themariofrancia themariofrancia requested review from a team as code owners May 21, 2026 11:05
@themariofrancia themariofrancia changed the title Fix/find 013 fix: transferring ownership of pre-existing deployemnts May 21, 2026
@themariofrancia themariofrancia self-assigned this May 21, 2026
@themariofrancia themariofrancia added the Audit Issues resulting from a code or process audit label May 21, 2026
}
if (transferOwnership & (1 << 2) != 0) {
(Ownable(address(irs))).transferOwnership(_tokenDetails.owner);
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please, use more than 3 words identifiers.
Create new functions to manipulate transferOwnership function and return true/false depending of the boolean to be check. Don't use magic numbers, create explicit constants with the functional meaning of each position on the uint256.

Function very complex, refatorize it to be clearer.

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

Labels

Audit Issues resulting from a code or process audit

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants