feat: integrate new genvm#599
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #599 +/- ##
==========================================
+ Coverage 18.61% 19.06% +0.45%
==========================================
Files 123 127 +4
Lines 9618 9759 +141
Branches 299 300 +1
==========================================
+ Hits 1790 1861 +71
- Misses 7744 7813 +69
- Partials 84 85 +1 ☔ View full report in Codecov by Sentry. 🚨 Try these New Features:
|
|
|
||
| if __name__ == "__main__": | ||
| app.run(debug=True, port=os.environ.get("SELENIUMPORT"), host="0.0.0.0") | ||
| app.run(debug=True, port=os.environ.get("WEBREQUESTPORT"), host="0.0.0.0") |
Check failure
Code scanning / CodeQL
Flask app is run in debug mode
f503f68 to
1fe8c32
Compare
denishacquin
left a comment
There was a problem hiding this comment.
Awesome refactoring of the redundant logic between constructors and methods! Love it. Added a few preliminary questions and comments, have a look when you can and later I'll try to set up the branch locally like you described to do some testing locally. @kp2pml30
ecec85a to
d377c99
Compare
|
I also totally don't understand what pre-commit wants from me, I haven't even changed that file and |
probably it was left broken from another person (it's not a requirement to merge) |
|
@kirilaa I'm getting this error on the console when opening a contract EDITED: the right person to tag would be @kp2pml30 |
|
@kirilaa probably related to my prior question: do we need to install the GenVM? How can we package it so that users can use the studio (simulator)? It'd be great if it was on a container, but it could also be a script to be added to the genlayer-init cli EDITED: the right person to tag would be @kp2pml30 |
AgustinRamiroDiaz
left a comment
There was a problem hiding this comment.
This is a partial review, I'll continue tomorrow
There was a problem hiding this comment.
are we repurposing the webrequest module to be like the node interfaces?
There was a problem hiding this comment.
Well, it is really repurposed for reuse of all llms (genvm natively supports less providers right now), while original selenium python wrapper was removed (and replaced by pure selenium with rest API)
There was a problem hiding this comment.
It'd be great if we had some renaming or documentation to know that. Maybe a short description of the architecture
|
@AgustinRamiroDiaz please see updated slack message
added it to the pr description as well |
|
@kp2pml30 can |
| decodedData: { | ||
| functionName: method, | ||
| args, | ||
| ...args, |
There was a problem hiding this comment.
NOTE: this caused a type error, transaction type was adjusted as well
| @@ -1,20 +1,25 @@ | |||
| from backend.node.genvm.icontract import IContract | |||
| # { "Depends": "py-genlayer:test" } | |||
There was a problem hiding this comment.
@kp2pml30 is this just a comment or is this something that the GenVM uses?
There was a problem hiding this comment.
Ok so we need to add this line on some contracts. Do we have some documentation for general Intelligent Contracts implementation?
There was a problem hiding this comment.
No we don't have it in any public repos. Where should I put it?
There was a problem hiding this comment.
There was a repo for "Marvin" documentation: https://github.com/yeagerai/marvin but right now I think that just a Google Doc with the basic stuff would be great to explain that to other devs that are using the simulator today :)
There was a problem hiding this comment.
Putting docs in private repos is sort of useless...
There was a problem hiding this comment.
of course, but they aren't if we release them at some point.
On the other hand, at least the internal team should be able to understand how to write intelligent contracts fully.
There was a problem hiding this comment.
For example this line # { "Depends": "py-genlayer:test" } how can I know when and how to use it?
…hat rose exception
| def get_transaction_by_hash( | ||
| transactions_processor: TransactionsProcessor, transaction_hash: str | ||
| ) -> dict: | ||
| ) -> dict | None: |
There was a problem hiding this comment.
This one caused typing exception
jsonrpc-1 | ERROR | jsonrpc error
jsonrpc-1 | Traceback (most recent call last):
jsonrpc-1 | File "/usr/local/lib/python3.12/site-packages/flask_jsonrpc/site.py", line 186, in dispatch
jsonrpc-1 | resp_view = current_app.ensure_sync(view_func)(*params)
jsonrpc-1 | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
jsonrpc-1 | File "/usr/local/lib/python3.12/site-packages/asgiref/sync.py", line 254, in __call__
jsonrpc-1 | return call_result.result()
jsonrpc-1 | ^^^^^^^^^^^^^^^^^^^^
jsonrpc-1 | File "/usr/local/lib/python3.12/concurrent/futures/_base.py", line 449, in result
jsonrpc-1 | return self.__get_result()
jsonrpc-1 | ^^^^^^^^^^^^^^^^^^^
jsonrpc-1 | File "/usr/local/lib/python3.12/concurrent/futures/_base.py", line 401, in __get_result
jsonrpc-1 | raise self._exception
jsonrpc-1 | File "/usr/local/lib/python3.12/site-packages/asgiref/sync.py", line 331, in main_wrap
jsonrpc-1 | result = await self.awaitable(*args, **kwargs)
jsonrpc-1 | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
jsonrpc-1 | File "/usr/local/lib/python3.12/site-packages/typeguard/__init__.py", line 1055, in async_wrapper
jsonrpc-1 | check_return_type(retval, memo)
jsonrpc-1 | File "/usr/local/lib/python3.12/site-packages/typeguard/__init__.py", line 840, in check_return_type
jsonrpc-1 | raise TypeError(*exc.args) from None
jsonrpc-1 | TypeError: type of the return value must be dict; got NoneType instead
AgustinRamiroDiaz
left a comment
There was a problem hiding this comment.
Changes look good in general. I don't understand 100% of the code, but I think it'll make more sense for me in the future, once the behavior of the studio matches the expected behavior
Please create GitHub issues for all FIXME and TODOs
|
@kp2pml30 please don't merge if e2e tests don't pass |
|
Please fix this error (when deploying an IC) prior to merge |
…ulator into integrate-genvm
|
🎉 This PR is included in version 0.22.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |


Fixes <no issue>
What
Why
Testing done
Decisions made
readmethods are hacky because currently I haven't implemented calldata parsing in the frontend. For that reason it returns it in a string format, which is partially json compatible. Tests also use that factChecks
Reviewing tips
@denishacquin please review frontend changes, I tested them by hand but still
Even thought I reread this PR myself, changes are massive and there may be something I missed, feel free to ask any questions and suggest changes
WEBREQUESTSELENIUMPORT = '5001'GENVM_BIN = "/genvm/bin"was added to.env.exampleand so it should appear in your.envas wellUser facing release notes
Users must know that new genvm looks stylish and cool 😎