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.

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

feat: improve p/ownable API #2330

Merged
merged 5 commits into from
Jun 13, 2024
Merged

Conversation

moul
Copy link
Member

@moul moul commented Jun 11, 2024

Improve the p/ownable API based on my needs encountered recently.

  • switch from origcaller to prevrealm
  • add AssertXXX helper
  • add a new constructor taking an arbitrary address
  • improve coding style in tests
  • identified a new bug with std.TestSetRealm (cc @thehowl)

Part of #2150

Signed-off-by: moul <94029+moul@users.noreply.github.com>
@moul moul self-assigned this Jun 11, 2024
@github-actions github-actions bot added the 🧾 package/realm Tag used for new Realms or Packages. label Jun 11, 2024
Signed-off-by: moul <94029+moul@users.noreply.github.com>
@linhpn99
Copy link
Contributor

linhpn99 commented Jun 11, 2024

hey @moul, we still have this pr #2198 related which is under review

@moul moul marked this pull request as ready for review June 11, 2024 18:26
@moul moul requested a review from leohhhn as a code owner June 11, 2024 18:26
@moul
Copy link
Member Author

moul commented Jun 11, 2024

@linhpn99, I've left several comments. Your implementation is not yet ready.

You'll need to update your pull request based on these new additions.

Signed-off-by: moul <94029+moul@users.noreply.github.com>
Copy link
Contributor

@leohhhn leohhhn left a comment

Choose a reason for hiding this comment

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

This was on my mind for some time now - thanks for adding this.

I think we should also add event support for TransferOwnership & DropOwnership while we're at it.
This could be simply one event, OwnershipTransfer, which would show the new owner, and in case of DropOwnership, would emit an empty address for the new owner.

examples/gno.land/p/demo/ownable/ownable_test.gno Outdated Show resolved Hide resolved
Signed-off-by: moul <94029+moul@users.noreply.github.com>
@moul
Copy link
Member Author

moul commented Jun 12, 2024

Added std.Emit. But I don't know how to test them from unit tests.

@leohhhn
Copy link
Contributor

leohhhn commented Jun 12, 2024

@moul

Made an issue/discussion on testing emitted events: #2338

@leohhhn leohhhn merged commit b89a9c9 into gnolang:master Jun 13, 2024
11 checks passed
zivkovicmilos added a commit that referenced this pull request Jul 8, 2024
This PR introduces the `r/gnoland/monit` realm, which can be used by an
external tool to verify if everything is working well, including:
- gnokey compatibility (and all the tx/amino/etc)
- networking (rpc)
- realm state persistency (counter should be higher than the previous
value)

In addition to being a good target for an external monitoring agent, the
realm displays (`Render`) some information, including whether the agent
appears to be missing.

- [x] improve ownable (depends on #2330)
- [x] p/demo/watchdog
- [x] r/gnoland/monit
- [ ] ~update contribs/autocounterd~ -> let's @gnolang/devops tackle
this in another PR. -> #1443

---------

Signed-off-by: moul <94029+moul@users.noreply.github.com>
Co-authored-by: Miloš Živković <milos@zmilos.com>
gfanton pushed a commit to gfanton/gno that referenced this pull request Jul 23, 2024
This PR introduces the `r/gnoland/monit` realm, which can be used by an
external tool to verify if everything is working well, including:
- gnokey compatibility (and all the tx/amino/etc)
- networking (rpc)
- realm state persistency (counter should be higher than the previous
value)

In addition to being a good target for an external monitoring agent, the
realm displays (`Render`) some information, including whether the agent
appears to be missing.

- [x] improve ownable (depends on gnolang#2330)
- [x] p/demo/watchdog
- [x] r/gnoland/monit
- [ ] ~update contribs/autocounterd~ -> let's @gnolang/devops tackle
this in another PR. -> gnolang#1443

---------

Signed-off-by: moul <94029+moul@users.noreply.github.com>
Co-authored-by: Miloš Živković <milos@zmilos.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🧾 package/realm Tag used for new Realms or Packages.
Projects
Status: Done
Status: ✅ Done
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants