-
Notifications
You must be signed in to change notification settings - Fork 409
chore(ownable): separate current & previous checks #4278
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
base: master
Are you sure you want to change the base?
Conversation
🛠 PR Checks SummaryAll Automated Checks passed. ✅ Manual Checks (for Reviewers):
Read More🤖 This bot helps streamline PR reviews by verifying automated checks and providing guidance for contributors and reviewers. ✅ Automated Checks (for Contributors):🟢 Maintainers must be able to edit this pull request (more info) ☑️ Contributor Actions:
☑️ Reviewer Actions:
📚 Resources:Debug
|
origin := std.OriginCaller() | ||
previous := std.PreviousRealm() | ||
if origin != previous.Address() { | ||
panic("NewWithOrigin() should be called from init() where std.PreviousRealm() is origin") | ||
} |
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.
I believe this check is sensible as it protects against intentional misuse.
I prefer to keep it and expect those who do not want this extra check and are willing to take more risks to manually call std.origincaller.
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.
How about then just calling this function New
?
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.
it only works in init(); New should work from so many places
NewWithOrigin // init
New(addr) // default explicit
NewWithCurrent()
NewWithPrevious()
This PR adds a txtar testing the journey of typical users on gnoland. It also fixes some crossing issues in certain Realms. Depends on #4278 --------- Co-authored-by: Manfred Touron <[email protected]>
The discussion on this is; ownable is not a very gno-like library- its a concept taken from Solidity. We should try replacing this with newer, better libraries, such as authz. Ideally, we use ownable only for embedding it into objects; so maybe it's worth considering renaming this library to something else, so people do not automatically start with the premise of Solidity's ownable. |
This PR adds a txtar testing the journey of typical users on gnoland. It also fixes some crossing issues in certain Realms. Depends on gnolang#4278 --------- Co-authored-by: Manfred Touron <[email protected]>
Description
Re-adds
_ByPrevious
methods, cleans up tests.Need to fix the following:
These realms were definitely not fixed properly in the meta PRs. The tests are passing but the functionality is wrong.