redstone-finance / redstone-evm-connector

A tool to inject Redstone data into evm-compatible blockchains
19 stars 4 forks source link

Return wrapped contracts as promise #22

Closed 0xnook closed 2 years ago

0xnook commented 2 years ago

Hello, I upped the version of your library from 1.3.8 to 2.1.2, and noticed that calling WrapperBuilder.wrapLite(someContract).usingPriceFeed('redstone-stocks') takes longer than before (I assume this is due to the streamr subscriptions) and blocks the whole JS UI im building until they load (I wrap 3 contracts simultaneously).

Is there any way this can be done in a snappier way? Perhaps by having a method the returns of the wrapped contracts as a promise.

hatskier commented 2 years ago

Hey @0xnook thank you for creating this issue. We definitely need to fix it ASAP. Unfortunately, it would be difficult to change it to Promise because we don't call any async operations in the wrapLite method. But I will be happy to analyze the performance issues and fix them. Can you please share your code so I can try to reproduce the problem?

0xnook commented 2 years ago

Hey @hatskier, the project I'm working on is going to be open sourced soon, but not yet! (a bit self conscious about my code, prefer to ship something more ready hehe).

In the meantime I created this repo with a minimal example https://github.com/0xnook/wrapperTest, where clicking the button wraps 2 contracts using the evm-connector, and a 1s increment counter is shown to showcase the blocking behavior.

Thank you so much for taking a look!

devFromRedstone commented 2 years ago

Hey @0xnook thank you very much for preparing the wrapperTest example. It helped me to identify the problem. Unfortunately it can't be solved by converting to an async function, because of the streamr limitations. I will try to find an alternative solution and speed it up. I think there are also ways to improve the redstone-evm-connector code, because currently it creates a new streamr-client instance for each streamr source in each wrapped contract.

Temporary workaround

For now you can simply pass your own priceFeed configuration (without passing the streamr or streamr-storage sources). You can check the example configurations here and an example code that connects a custom configuration here.

I'll let you know once I fix the performance issues with streamr in a better way (approx. 1-2 weeks)

hatskier commented 2 years ago

@0xnook I've found the way to solve it. Will publish the fix during next 2-3 days and let you know :)

0xnook commented 2 years ago

Awesome guys, thanks for the fix and hope we can continue working together!

Our frontend is now open source, and hope it can serve as a reference implementation of your library https://github.com/PipilaDAO/frontend

hatskier commented 2 years ago

Awesome guys, thanks for the fix and hope we can continue working together!

Our frontend is now open source, and hope it can serve as a reference implementation of your library https://github.com/PipilaDAO/frontend

For sure! The fix has just been added to the version 2.1.4 of redstone-evm-connector

0xnook commented 2 years ago

Can confirm the new changes work like a charm and performance issues are fixed :smile: