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!: upgrade to contracts v0.12.0 + add tokenIdType to LSP8 deployment #226

Merged
merged 16 commits into from
Nov 3, 2023

Conversation

CJ42
Copy link
Member

@CJ42 CJ42 commented Oct 18, 2023

What kind of change does this PR introduce (bug fix, feature, docs update, ...)?

  • Upgrade @erc725/erc725.js to v0.20.0 + @lukso/lsp-smart-contracts to v0.12.0

⚠️ BREAKING CHANGES

  • add tokenIdType on deployment of LSP8 contract.
  • Remove any hardcoded literal constants related to the lsp-smart-contracts, but instead import the values directly from the @lukso/lsp-smart-contracts package.

🧪 Tests

  • Simplify the tests to use getData instead of getDataBatch when possible.

@CJ42 CJ42 marked this pull request as ready for review October 18, 2023 23:34
@CJ42 CJ42 changed the title refactor: upgrade @erc725/erc725.js + @lukso/lsp-smart-contracts to latest versions refactor!: upgrade to contracts v0.12.0 + add tokenIdType to LSP8 deployment Oct 18, 2023
@codecov-commenter
Copy link

codecov-commenter commented Oct 18, 2023

Codecov Report

Merging #226 (1a4e210) into develop (edb0c5e) will decrease coverage by 0.55%.
Report is 43 commits behind head on develop.
The diff coverage is 16.81%.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

@@             Coverage Diff             @@
##           develop     #226      +/-   ##
===========================================
- Coverage    19.72%   19.18%   -0.55%     
===========================================
  Files           16       15       -1     
  Lines         1034     1058      +24     
  Branches       358      371      +13     
===========================================
- Hits           204      203       -1     
- Misses         808      832      +24     
- Partials        22       23       +1     
Files Coverage Δ
src/lib/classes/lsp8-identifiable-digital-asset.ts 19.14% <ø> (ø)
src/lib/helpers/config.helper.ts 100.00% <100.00%> (ø)
src/lib/helpers/erc725.helper.ts 60.00% <100.00%> (+4.44%) ⬆️
src/lib/helpers/uploader.helper.ts 42.85% <0.00%> (ø)
src/lib/lsp-factory.ts 16.66% <0.00%> (ø)
src/lib/services/base-contract.service.ts 9.09% <75.00%> (+1.68%) ⬆️
src/lib/services/universal-receiver.service.ts 23.33% <33.33%> (ø)
src/lib/services/key-manager.service.ts 17.14% <0.00%> (-0.51%) ⬇️
src/lib/services/digital-asset.service.ts 7.10% <9.52%> (+0.37%) ⬆️
src/lib/helpers/deployment.helper.ts 22.13% <9.09%> (-4.25%) ⬇️
... and 1 more

📣 Codecov offers a browser extension for seamless coverage viewing on GitHub. Try it in Chrome or Firefox today!

Copy link
Contributor

@CallumGrindle CallumGrindle left a comment

Choose a reason for hiding this comment

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

nice thanks! Can we make the LSP8 tokenID type default to some value if not provided?

Also we will need the new verified base contract addresses in the versions.json file

Copy link
Member

@magalimorin18 magalimorin18 left a comment

Choose a reason for hiding this comment

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

lgtm

.prettierrc Show resolved Hide resolved
test/test.utils.ts Show resolved Hide resolved
src/lib/helpers/config.helper.ts Outdated Show resolved Hide resolved
src/lib/services/base-contract.service.ts Show resolved Hide resolved
src/lib/classes/universal-profile.spec.ts Outdated Show resolved Hide resolved
src/lib/classes/universal-profile.spec.ts Outdated Show resolved Hide resolved
src/lib/classes/universal-profile.spec.ts Outdated Show resolved Hide resolved
src/lib/classes/universal-profile.spec.ts Outdated Show resolved Hide resolved
src/lib/classes/proxy-deployer.spec.ts Outdated Show resolved Hide resolved
src/lib/classes/proxy-deployer.spec.ts Show resolved Hide resolved
@richtera
Copy link
Contributor

@CJ42 Can you fix the lint errors? The import order plugin is a bit particular in terms of the sequence and separation of various imports. They can be fixed in vscode or with --fix in eslint.

@CJ42 CJ42 force-pushed the refactor/upgrade branch 2 times, most recently from bcf18e4 to 143ea7c Compare October 19, 2023 16:06
src/versions.json Outdated Show resolved Hide resolved
@CallumGrindle CallumGrindle merged commit 22f778c into develop Nov 3, 2023
1 check passed
@CallumGrindle CallumGrindle deleted the refactor/upgrade branch November 3, 2023 11:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

5 participants