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

Refactor CIp3 functions into @celo/claims #15

Closed
aaronmgdr opened this issue Oct 18, 2023 · 10 comments · Fixed by #340
Closed

Refactor CIp3 functions into @celo/claims #15

aaronmgdr opened this issue Oct 18, 2023 · 10 comments · Fixed by #340

Comments

@aaronmgdr
Copy link
Member

aaronmgdr commented Oct 18, 2023

Currently Contractkit has a folder called identity with functions and methods for making cip3 metadata claims.

it doesn't feel like core part of contract kit.

Furthermore the @celo/identity package which is ostensibly part of social-connect has functions related to cip3 not social-connect

proposal

move the functions to new @celo/claims package which will be reponsible for soley for cip3 actions.

how this helps

by moving cip3 functions out of @celo/identity (and social-connect commands to new cli) celocli will not need to depend on celo/identity which due to the 2 monorepo having cross dependencies leads to issues

dependents

  • CLI
  • celo tool
  • celo/identity package

related

@aaronmgdr aaronmgdr self-assigned this Oct 18, 2023
@arthurgousset
Copy link
Contributor

arthurgousset commented Oct 18, 2023

Thanks for sharing 💯

Currently Contractkit has a folder called with functions and methods for making metadata claims.

Could you link to that folder for reference? Would love to take a look. Thanks!

@aaronmgdr
Copy link
Member Author

Thanks for sharing 💯

Currently Contractkit has a folder called with functions and methods for making metadata claims.

Could you link to that folder for reference? Would love to take a look. Thanks!

https://github.com/celo-org/celo-monorepo/tree/master/packages/sdk/contractkit/src/identity

@mcortesi
Copy link
Contributor

i'd check metadata-crawler, since i think this is not longer used and potentially could be archived, so not sure if it's a good home for identity pkg.

cc @lvpeschke @alecps

@aaronmgdr
Copy link
Member Author

@mcortesi In that case maybe all this could just be deleted.

I looked a few months ago and the crawler was used I think by blockscout?

AFAIK this is the code that allows validators to name themselves (or really any address to have a name associated with it)

@lvpeschke
Copy link
Contributor

metadata-crawler should be removed, but I'm not yet clear on all the dependencies; this means we should not add to it if it can be avoided.

@aaronmgdr
Copy link
Member Author

i think i wasnt clear here. Im not proposing to add to metadata crawler, im proposing a new package that IF metadata crawler lives would probably be its neighbour

@aaronmgdr aaronmgdr changed the title Reorg: Move contractkit/identity to metadata Reorg: Move contractkit/identity to new metadata package Oct 20, 2023
@aaronmgdr aaronmgdr changed the title Reorg: Move contractkit/identity to new metadata package Reorg: Move contractkit/identity to new claims package Oct 20, 2023
@lvpeschke
Copy link
Contributor

Then I'm not opposed, detangling pieces that don't need to be mixed together makes it easier to kill them off one by one. :)

@arthurgousset
Copy link
Contributor

arthurgousset commented Oct 20, 2023

Thanks for the link @aaronmgdr.
It looks like contractkit/src/identity relates to:

@arthurgousset
Copy link
Contributor

+1 to

detangling pieces that don't need to be mixed together

@aaronmgdr
Copy link
Member Author

Ok so CIP-3 related functions can move to @celo/metadata-claims,

CIP-8 functions are either marked deprecated or just straight up removed (how aggressive do we want to be)

@aaronmgdr aaronmgdr removed their assignment Nov 9, 2023
@aaronmgdr aaronmgdr transferred this issue from celo-org/celo-monorepo Dec 14, 2023
@aaronmgdr aaronmgdr changed the title Reorg: Move contractkit/identity to new claims package Refactor CIp3 functions into @celo/claims Apr 16, 2024
aaronmgdr added a commit that referenced this issue Oct 2, 2024
### Description

@celo/contractkit has for a long time basically had within it a
subpackage under /identity that really didnt have anything to do with
contractkit. Removing it will make it easier to build social connect
tools as they use the IdentityMetadataMapper contained in here. However
this is not social connect tool so it will remain in the developer
tooling repo.

This is a prereq to remove contractkit as a dependency for social
connect. a long term aim.

### Other changes


### Tested

uses existing tests

### Related issues

- Fixes #15

### Backwards compatibility
nope
exports moved to a new package. file structure slightly altered.
parameter for IdentityMetadataMapper changes to take an object with
specific functions rather than a kit
### Documentation

<!-- start pr-codex -->

---

## PR-Codex overview
This PR introduces the `@celo/metadata-claims` package, extracting
identity-related functionality from `@celo/contractkit`. This allows
developers to use the `IdentityMetadataWrapper` independently of
`ContractKit`, enhancing modularity and usability.

### Detailed summary
- Created `@celo/metadata-claims` package.
- Moved identity and claims-related functionality from
`@celo/contractkit`.
- Updated imports in CLI commands and SDK files to use the new package.
- Introduced types like `AccountMetadataSignerGetters`.
- Removed identity-related exports from `@celo/contractkit`.
- Added tests for new functionality in the `metadata-claims` package.

> The following files were skipped due to too many changes:
`packages/sdk/wallets/wallet-rpc/lib/rpc-signer.js.map`,
`packages/sdk/wallets/wallet-rpc/lib/rpc-signer.js`,
`packages/sdk/wallets/wallet-rpc/lib/rpc-wallet.test.js.map`,
`docs/sdk/metadata-claims/modules/claim.md`,
`packages/sdk/wallets/wallet-rpc/lib/rpc-wallet.test.js`,
`docs/sdk/metadata-claims/classes/metadata.IdentityMetadataWrapper.md`

> ✨ Ask PR-Codex anything about this PR by commenting with `/codex {your
question}`

<!-- end pr-codex -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants