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

Reintroduce domain deprecation #1280

Merged
merged 15 commits into from
Oct 11, 2024
Merged

Reintroduce domain deprecation #1280

merged 15 commits into from
Oct 11, 2024

Conversation

kronosapiens
Copy link
Member

@kronosapiens kronosapiens commented Jul 24, 2024

Closes #1279

This PR restores the ability to deprecate domains and local skills. It also moves skills deprecation entirely to the colony level and away from the network, completing the process begun in #1173.

There are two migration considerations, both of which I believe are relatively minor:

  • The current localSkills boolean mapping is deprecated and replaced by a LocalSkill struct mapping. For already-extant local skills, validity and deprecation behaviors are preserved.
  • Due to the "deprecation of deprecation" on the colony network, all currently-deprecated skills will become "undeprecated". Affected users will need to re-deprecate any skills or domains such affected.

The point to emphasize is that these migration issues affect functionality that has been deprecated for some time. As such, the actual effects are likely to be very minor.

TODO:

  • Add upgrade test for local skill deprecation

@kronosapiens kronosapiens force-pushed the maint/deprecate-domains branch 6 times, most recently from f9dc857 to d1bc2ec Compare July 25, 2024 21:38
@kronosapiens kronosapiens marked this pull request as ready for review July 25, 2024 21:39
@kronosapiens kronosapiens force-pushed the maint/deprecate-domains branch 4 times, most recently from e01599d to 030da01 Compare July 26, 2024 18:43
@area
Copy link
Member

area commented Jul 29, 2024

What on earth has happened to .storage-layouts-normalized?

@kronosapiens kronosapiens force-pushed the maint/deprecate-domains branch 2 times, most recently from e58aee8 to 5f60345 Compare July 29, 2024 21:30
@area area force-pushed the maint/deprecate-domains branch from 5f60345 to 5ca6132 Compare August 5, 2024 11:06
@kronosapiens kronosapiens force-pushed the maint/deprecate-domains branch from 1a55020 to 22a012b Compare August 5, 2024 18:18
contracts/colony/Colony.sol Outdated Show resolved Hide resolved
contracts/colonyNetwork/IColonyNetwork.sol Show resolved Hide resolved
contracts/colonyNetwork/ColonyNetworkSkills.sol Outdated Show resolved Hide resolved
@kronosapiens kronosapiens force-pushed the maint/deprecate-domains branch from cc726a3 to 9376b82 Compare August 13, 2024 18:15
contracts/colonyNetwork/IColonyNetwork.sol Outdated Show resolved Hide resolved
contracts/colony/Colony.sol Outdated Show resolved Hide resolved
@kronosapiens kronosapiens force-pushed the maint/deprecate-domains branch from e4703a8 to a539a2a Compare September 4, 2024 21:44
Copy link
Member

@area area left a comment

Choose a reason for hiding this comment

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

Implementation looks good. Just need to work out how to convince myself further that it won't brick everything with the return data change when calling getDomain.

test/contracts-network/colony-expenditure.js Outdated Show resolved Hide resolved
test/contracts-network/colony.js Show resolved Hide resolved
@kronosapiens kronosapiens force-pushed the maint/deprecate-domains branch from a539a2a to f33bc54 Compare September 11, 2024 14:09
@area
Copy link
Member

area commented Sep 13, 2024

I spent this afternoon getting extremely comfortable with Remix and EVM Opcodes to convince myself further that returning too much data in a call is okay, and I believe it is. I might end up adding to the relevant OneTxPayment test though.

I also don't believe bricking the mining cycle is a risk after all, as the only getDomain() call is in the initialization, so that makes me a little happier. I still don't want to merge this until after multisig has been merged and deployed, though.

@kronosapiens kronosapiens force-pushed the maint/deprecate-domains branch from 6ecae63 to c6d9a09 Compare September 16, 2024 18:34
@area
Copy link
Member

area commented Sep 27, 2024

Still waiting on multisig.

That's now merged, but not yet deployed... so closer, at least!

@area
Copy link
Member

area commented Oct 1, 2024

Needs #1299 merged, and then versions bumped accordingly.

@area area force-pushed the maint/deprecate-domains branch from e3368e6 to 24c6e80 Compare October 2, 2024 06:29
@area area force-pushed the maint/deprecate-domains branch from 24c6e80 to a073fc3 Compare October 7, 2024 15:06
@area area merged commit ce8ca4a into develop Oct 11, 2024
2 checks passed
@area area deleted the maint/deprecate-domains branch October 11, 2024 15:17
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.

Archiving domains
2 participants