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: add wallet messaging #53

Merged
merged 77 commits into from
Nov 29, 2023
Merged

feat: add wallet messaging #53

merged 77 commits into from
Nov 29, 2023

Conversation

jrriehl
Copy link
Contributor

@jrriehl jrriehl commented Feb 10, 2023

Adds wallet messaging as an optional feature. This means it needs to be installed with:

  • pip install uagents[wallet]
  • pip install uagents[all]
  • poetry install -E wallet
  • poetry install -E all

Adds on_wallet_message decorator and uses babble client to send and retrieve messages from wallet messaging server.

Example 15-wallet-messaging demonstrates the new feature.

.pylintrc Outdated Show resolved Hide resolved
pyproject.toml Outdated Show resolved Hide resolved
src/uagents/agent.py Outdated Show resolved Hide resolved
@jrriehl jrriehl marked this pull request as ready for review March 15, 2023 14:33
@Archento
Copy link
Member

I can't poetry install frozenlist and pyyaml. This may be due to my pipx venv using Python3.12
Are these mandatory and is there a way to test python compatibility issues?

@jrriehl
Copy link
Contributor Author

jrriehl commented Nov 27, 2023

I can't poetry install frozenlist and pyyaml. This may be due to my pipx venv using Python3.12 Are these mandatory and is there a way to test python compatibility issues?

We certainly don't claim to support Python 3.12 yet. Can you try with an earlier version for now and then we can raise an issue to fix this?

Actually up until this PR, we still specified python ">=3.8, <3.12", but this is an annoying constraint to enforce when installing with other packages, so I think "^3.8" is better and maybe this will also push us to stay up to date with the latest Python versions.

@Archento
Copy link
Member

Archento commented Nov 27, 2023

We certainly don't claim to support Python 3.12 yet. Can you try with an earlier version for now and then we can raise an issue to fix this?

When forcing 3.11.6 with all extensions installed I still get errors but this time from coincurve (didn't we got rid of that a while ago?)

Also my Poetry installation strangely is hardwired to 3.12 and when using brew on Mac I am not able to specify which version to install (that poetry uses for installations) Meaning that when I install Poetry itself on my system it implicitly installs and uses the latest registered python version which is 3.12.
The virtual environment itself can be switched to 3.11 but using poetry install still throws an error where it suddenly uses 3.12...
I'm going to try some other options, but if that doesn't help then I'm stuck

@jrriehl
Copy link
Contributor Author

jrriehl commented Nov 27, 2023

We certainly don't claim to support Python 3.12 yet. Can you try with an earlier version for now and then we can raise an issue to fix this?

When forcing 3.11.6 with all extensions installed I still get errors but this time from coincurve (didn't we got rid of that a while ago?)

We did remove that from CosmPy but babble brings this back unfortunately. For me this is the biggest pain point and the main reason we wanted to make sure this was an optional package in uAgents.

To fix this on a Mac, you should be able to run brew install automake.

@ejfitzgerald Any chance we could do the same tricks to pull this coincurve dependency out of babble?

Update: Seems that this is doable, but not particularly easy and probably won't make it into this PR.

@Archento
Copy link
Member

Following the instructions on their page (docs) for MacOS:

xcode-select --install
brew install autoconf automake libffi libtool pkg-config python

I was able to install coincurve and since I already had the rest - it seems that one or all of 'libffi, libtool and pkg-config' were missing.

Copy link
Member

@Archento Archento left a comment

Choose a reason for hiding this comment

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

I have just a couple of remarks but all in all it's very good!

It works as intended (when the correct environment is present) and I am curious to see this in action in more complex agent scenarios. The only downside is that this implementation does not seamlessly fit into the existing uagents codebase or looking at it from a different angle it's the perfect example for an add-on/extra module.

python/src/uagents/wallet_messaging.py Show resolved Hide resolved
python/src/uagents/context.py Outdated Show resolved Hide resolved
python/src/uagents/agent.py Show resolved Hide resolved
python/src/uagents/agent.py Show resolved Hide resolved
python/src/uagents/wallet_messaging.py Show resolved Hide resolved
Copy link
Member

@Archento Archento left a comment

Choose a reason for hiding this comment

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

All good from my side now!

@jrriehl jrriehl merged commit 98f8df5 into main Nov 29, 2023
8 checks passed
@jrriehl jrriehl deleted the babble branch November 29, 2023 10:20
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 this pull request may close these issues.

3 participants