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

xcmsend-m1 #1018

Merged
merged 2 commits into from
Oct 6, 2023
Merged

xcmsend-m1 #1018

merged 2 commits into from
Oct 6, 2023

Conversation

flipchan
Copy link
Contributor

Milestone Delivery Checklist

  • The milestone-delivery-template.md has been copied and updated.
  • This pull request is being made by the same account as the accepted application.
  • I have disclosed any and all sources of reused code in the submitted repositories and have done my due diligence to meet its license requirements.
  • In case of acceptance, an invoice must be submitted and the payment will be transferred to the BTC/ETH/fiat account provided in the application.
  • The delivery is according to the Guidelines for Milestone Deliverables.

Link to the application pull request: w3f/Grants-Program#1937

@decentration
Copy link
Contributor

decentration commented Oct 3, 2023

@decentration
Copy link
Contributor

Hey reviewer here is a video for you:
https://www.loom.com/share/e3888c885e60421390212ae8d276d753?sid=0ce4ccbe-a0e9-485d-8961-436f6a4118d0

@semuelle semuelle self-assigned this Oct 4, 2023
@decentration
Copy link
Contributor

hey there @takahser here is a review if you are available? 👍

@takahser
Copy link
Contributor

takahser commented Oct 6, 2023

@flipchan @decentration thanks for the delivery. I think @semuelle might have already started reviewing it, so reviewing myself might not be the most efficient way forward. I recommend waiting for @semuelle to finish his review. 👍

Copy link
Member

@semuelle semuelle left a comment

Choose a reason for hiding this comment

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

Hi @decentration & @flipchan. I am reviewing the milestone right now. Sorry for the wait.

Thanks a lot for the video and the documentation. So far, everything looks great. The workflow is not great yet, but it looks very promising. You can find my evaluation notes here. Just one question: what are the tests actually testing? I understand that you are not planning to perform actual txs here, but it seems like you are not checking any return values either.

I also noted the UI issues I found, although I'm aware that you pointed them out in the video and that there is another milestone coming. Lastly, is there a reason you are not linking to your website or repository from the Medium article?

@semuelle
Copy link
Member

semuelle commented Oct 6, 2023

Also, I noticed that xcmsend.com is trying to connect to localhost. I'm guessing this is related to the authentication service?

image

@flipchan
Copy link
Contributor Author

flipchan commented Oct 6, 2023

@semuelle Thank you for your feedback, I have updated the tests(BagpipesOrg/app-v0.0.1#9) so now the tests check:

  • drafting transactions and checking that they are encoded in the right way
  • checking if each chain returns correct asset and native balance

Note(if you want to):
You can broadcast each tx, by feeding the tx into the broadcast_run_tx function

Tests have been pushed and can be found here:
https://github.com/XcmSend/app-v0.0.1/blob/main/src/tests/run_tests.ts

@flipchan
Copy link
Contributor Author

flipchan commented Oct 6, 2023

  • Lastly, is there a reason you are not linking to your website or repository from the Medium article?

XCMSend.com(note: this instance is out of scope for this grant application and is an extra feature) is put up for everyone that doesn't want to run it locally, it is linked in the documentation page: https://xcmsend.github.io/deployments/public.html

For now xcmsend.com is a test instance and we estimate that once we reach the second milestone we will be able to sync the latest master branch from the git repo to deploy it on the public instance. Right now its manually deployed on XCMSend.com and the site works, but we still consider this instance as a test instance.

If I can be of any further assistance, let me know! Thanks @semuelle

@decentration
Copy link
Contributor

the medium article with link to repo and site added

@flipchan flipchan requested a review from semuelle October 6, 2023 14:51
@decentration
Copy link
Contributor

@semuelle thanks for your review, if you also (extraneous to this review) have any feedback on what you would like to build with this kind of workflow builder, then it would be greatly welcomed and would inform what kind of features are prioritised. We're excited about throwing as many pallets we can into the playground, but we are still curious what new things users would build with it, and in what order they should be added.

Copy link
Member

@semuelle semuelle left a comment

Choose a reason for hiding this comment

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

Thanks for the quick reply, @decentration. No worries about the website link, I was just curious.
I updated my evaluation notes and your milestone is hereby accepted. Congratulations! I will forward your invoice for processing.

any feedback on what you would like to build with this kind of workflow builder

To be honest, my initial reaction was that this kind of UI is great for people who do a lot of repetitive transfers, e.g. payroll. People who are tech-savvy enough might just build a script or copy-paste into polkadot-js, but this drag-and-drop style is easy to understand and replicate. Alternatively, and I believe this came up in the application discussion already, this might be nice addition for a wallet.

@semuelle semuelle merged commit 6674675 into w3f:master Oct 6, 2023
3 checks passed
@decentration
Copy link
Contributor

@semuelle thanks for the pre-monday merge, and thanks for the feedback. Payroll is something we will look at more closely. and yes indeed "easy to understand and replicate" is a nice combo, we definitely want to make it easy for people to be creative in solving their own problems. And even myself as technical want to make it easier to compose transactions using xcm. I think the challenge for us is getting the balance between simple abstraction and complexity. 👍

@RouvenP
Copy link

RouvenP commented Oct 13, 2023

hi @flipchan we transferred the payment today

@flipchan
Copy link
Contributor Author

Thank you @RouvenP , payment is confirmed on our end

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.

5 participants