search

safe-global / safe-smart-account

Safe allows secure management of blockchain assets.
https://safe.global
GNU Lesser General Public License v3.0
1.88k stars 927 forks source link

Prefer `this` over `msg.sender` to refer to the safe #682

Closed voltrevo closed 1 year ago

voltrevo commented 1 year ago

msg.sender refers to the safe only because of the way safe's module system works via an internal call. (Related to how _msgSender() is needed to get the 'real' msg sender.)

To my knowledge, address(this) should be equivalent. If so, it should be preferred because it is much more clear.

github-actions[bot] commented 1 year ago

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

voltrevo commented 1 year ago

I have read the CLA Document and I hereby sign the CLA

voltrevo commented 1 year ago

Nevermind. I realize now that the plugin is used via normal calls except for the setup phase, and that means this refers to the plugin rather than the safe.

I'll leave this open to help prompt a response here which could help others. In particular, is there documentation that covers this point?

mmv08 commented 1 year ago

Nevermind. I realize now that the plugin is used via normal calls except for the setup phase, and that means this refers to the plugin rather than the safe.

I'll leave this open to help prompt a response here which could help others. In particular, is there documentation that covers this point?

I'm afraid we don't have any documentation covering this at the moment, but we're actively working on improving this. I'll close the PR in the meantime (it should still be accessible even if it's closed)