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: sim replace frontend rpc calls with genlayerjs #600

Merged
merged 34 commits into from
Dec 4, 2024

Conversation

denishacquin
Copy link
Contributor

@denishacquin denishacquin commented Nov 13, 2024

Fixes #580

What

  • Implemented GenLayer JS in the Studio frontend
  • Added possibility to return null on get_transaction_by_hash endpoint (was breaking when a server-side tx didn't exist)

Why

  • To get closer to the actual final implementation and use the correct api's
  • To fix a bug

Testing done

  • tested the new feature
  • tested the bug fix

Decisions made

  • Removed some functionality that is now covered by the GenLayer JS library (nonces, useWallet hook...)

Checks

  • I have tested this code
  • I have reviewed my own PR
  • I have created an issue for this PR
  • I have set a descriptive PR title compliant with conventional commits

User facing release notes

  • Implemented the official GenLayer JS library to handle accounts and transactions in the Studio

@denishacquin denishacquin linked an issue Nov 13, 2024 that may be closed by this pull request
Copy link

codecov bot commented Nov 13, 2024

Codecov Report

Attention: Patch coverage is 30.08850% with 79 lines in your changes missing coverage. Please review.

Project coverage is 18.74%. Comparing base (a1da20d) to head (267c809).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
frontend/src/hooks/useContractQueries.ts 10.71% 25 Missing ⚠️
frontend/src/hooks/useGenlayer.ts 26.47% 25 Missing ⚠️
frontend/src/hooks/useShortAddress.ts 5.55% 17 Missing ⚠️
frontend/src/components/Simulator/AccountItem.vue 0.00% 4 Missing ⚠️
frontend/src/components/Simulator/ContractInfo.vue 0.00% 3 Missing ⚠️
frontend/src/hooks/useSetupStores.ts 25.00% 3 Missing ⚠️
...rontend/src/components/Simulator/AccountSelect.vue 0.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #600      +/-   ##
==========================================
- Coverage   19.45%   18.74%   -0.71%     
==========================================
  Files         129      129              
  Lines        9968     9907      -61     
  Branches      317      307      -10     
==========================================
- Hits         1939     1857      -82     
- Misses       7944     7965      +21     
  Partials       85       85              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.


🚨 Try these New Features:

@denishacquin
Copy link
Contributor Author

@cristiam86 any hint on how to use leaderonly functionality with GenLayer JS?

@denishacquin denishacquin self-assigned this Nov 13, 2024
@cristiam86
Copy link
Collaborator

cristiam86 commented Nov 13, 2024

@cristiam86 any hint on how to use leaderonly functionality with GenLayer JS?

sure, to use it in GenLayerJS, it should be implemented there :D
yeagerai/genlayer-js#24

Den added 2 commits November 14, 2024 19:03
…ayerjs

# Conflicts:
#	frontend/src/components/Simulator/ContractInfo.vue
#	frontend/src/hooks/index.ts
#	frontend/src/hooks/useSetupStores.ts
#	frontend/src/stores/transactions.ts
#	frontend/test/unit/stores/transactions.test.ts
…ayerjs

# Conflicts:
#	frontend/src/hooks/index.ts
@denishacquin
Copy link
Contributor Author

Regarding the leaderOnly feature, I've been trying to implement it in genlayer-js, without success until now.

If I dare ask, how important is that feature? Won't it have to be removed once we hook the studio to the real chain? @cristiam86

@cristiam86
Copy link
Collaborator

Regarding the leaderOnly feature, I've been trying to implement it in genlayer-js, without success until now.

If I dare ask, how important is that feature? Won't it have to be removed once we hook the studio to the real chain? @cristiam86

Let's take a look together whenever you can :) This is going to be available permanently in the localnet. Of course, it won't be available in the testnet or mainnet.
In Ethereum and other L1s, there are some methods that can be only used in the localnet and they make it easy to force situations or to speed up the development.

@denishacquin denishacquin marked this pull request as ready for review November 19, 2024 13:59
frontend/src/hooks/useContractQueries.ts Outdated Show resolved Hide resolved
frontend/src/hooks/useContractQueries.ts Outdated Show resolved Hide resolved
@denishacquin
Copy link
Contributor Author

Note that if we want to support the leaderOnly param in this PR, we need to merge the feature in genlayer-js then update the dependency here. @cristiam86

@cristiam86
Copy link
Collaborator

Oh, sure, sorry, my bad.
@kp2pml30 can we go ahead refactoring this in the frontend with the new schema?

@kp2pml30
Copy link
Collaborator

kp2pml30 commented Nov 28, 2024

Sorry, I don't understand the question...

Current abi has types in frontend/src/types/index.ts

then these types are used in frontend/src/components/Simulator/ConstructorParameters.vue and frontend/src/components/Simulator/ContractMethodItem.vue

@denishacquin
Copy link
Contributor Author

Ok, then we simply need to update the ContractSchema type in genlayer-js to match the one in the studio.

@denishacquin
Copy link
Contributor Author

denishacquin commented Nov 28, 2024

Created an issue on the genlayer-js side, will take care of it:
yeagerai/genlayer-js#33

@denishacquin denishacquin marked this pull request as draft November 28, 2024 10:57
@denishacquin denishacquin marked this pull request as ready for review November 28, 2024 14:14
Copy link
Collaborator

@cristiam86 cristiam86 left a comment

Choose a reason for hiding this comment

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

Great work, but we need to merge this one first: yeagerai/genlayer-js#35 and update the leader_only parameter here.

@denishacquin
Copy link
Contributor Author

Great work, but we need to merge this one first: yeagerai/genlayer-js#35 and update the leader_only parameter here.

Went ahead and merged but turns out we forgot a couple - fixed here: #6 please review @cristiam86

As soon as it's released in 0.4.6 I'll bump the version here again!

Copy link

sonarqubecloud bot commented Dec 4, 2024

Quality Gate Failed Quality Gate failed

Failed conditions
0.0% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube Cloud

Copy link
Contributor Author

@denishacquin denishacquin left a comment

Choose a reason for hiding this comment

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

leader only was updated on both sides

Copy link
Contributor Author

@denishacquin denishacquin left a comment

Choose a reason for hiding this comment

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

leader only ok

@cristiam86 cristiam86 merged commit ff5fcb7 into main Dec 4, 2024
28 of 33 checks passed
@denishacquin denishacquin deleted the 580-sim-replace-frontend-rpc-calls-with-genlayerjs branch December 4, 2024 09:53
Copy link
Contributor

github-actions bot commented Dec 5, 2024

🎉 This PR is included in version 0.27.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SIM-Replace frontend RPC calls with GenlayerJS
3 participants