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(contract): add a burn function to destroy unused old sites #291

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

Tzal3x
Copy link
Collaborator

@Tzal3x Tzal3x commented Nov 8, 2024

  • Add a burn function to the contract.
  • Fix: The test_new_range_no_bounds_defined should fail since at least one range bound should have been defined.

The site-builder will be updated when there will also be a delete argument support where both the blobs on walrus and the sui objects of a site will be deleted.

Warning: The burn function should be called after the site-builder has deleted all dynamic fields (resources and routes) attached to the site! Otherwise, deleting an object that has dynamic fields still defined on it renders them all inaccessible to future transactions!

@Tzal3x Tzal3x self-assigned this Nov 8, 2024
@Tzal3x Tzal3x linked an issue Nov 8, 2024 that may be closed by this pull request
Copy link

vercel bot commented Nov 8, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
walrus-sites-sp ✅ Ready (Inspect) Visit Preview 💬 Add feedback Nov 15, 2024 0:20am
walrus-sites-sp-devnet-fallback ✅ Ready (Inspect) Visit Preview 💬 Add feedback Nov 15, 2024 0:20am
walrus-sites-sp-testnet ✅ Ready (Inspect) Visit Preview 💬 Add feedback Nov 15, 2024 0:20am
walrus-sites-sw ✅ Ready (Inspect) Visit Preview 💬 Add feedback Nov 15, 2024 0:20am
walrus-sites-sw-testnet ✅ Ready (Inspect) Visit Preview 💬 Add feedback Nov 15, 2024 0:20am

Copy link

codspeed-hq bot commented Nov 8, 2024

CodSpeed Performance Report

Merging #291 will not alter performance

Comparing 279-contracts-add-a-burn-function-to-destroy-unusedold-sites (4e7d71e) with main (c25cb04)

Summary

✅ 3 untouched benchmarks

Copy link
Collaborator

@giac-mysten giac-mysten left a comment

Choose a reason for hiding this comment

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

Thank you @Tzal3x ! Just a couple of suggestions

move/walrus_site/sources/site.move Outdated Show resolved Hide resolved
@@ -202,4 +202,15 @@ module walrus_site::site {
let routes = df::borrow_mut(&mut site.id, ROUTES_FIELD);
routes_remove(routes, route)
}

/// Deletes a site object.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Question: Can we expose a function to delete everything?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Everything meaning the dynamic fields? I looked into it, and I couldn't find a way to get all the dynamic fields on the contract level. 😬 So this seems to be unavoidable. Related thread: https://mysten-labs.slack.com/archives/C068VUPSG02/p1730982934293609.

move/walrus_site/tests/site_tests.move Show resolved Hide resolved
move/walrus_site/Move.lock Outdated Show resolved Hide resolved
@Tzal3x Tzal3x changed the base branch from main to testnet November 12, 2024 10:09
@Tzal3x Tzal3x changed the base branch from testnet to main November 13, 2024 16:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Contracts: add a burn function to destroy unused/old sites
2 participants