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: subxt script to fund dev accounts via XCM #53

Merged
merged 10 commits into from
Apr 2, 2024

Conversation

peterwht
Copy link
Collaborator

@peterwht peterwht commented Mar 27, 2024

#47

@peterwht peterwht marked this pull request as ready for review March 27, 2024 05:36
Copy link
Collaborator

@Daanvdplas Daanvdplas left a comment

Choose a reason for hiding this comment

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

Nicely done! One thing to add is three queries to Pop Network to ensure that alice, bob and charlie have received the funds.

Cargo.toml Outdated Show resolved Hide resolved
scripts/fund-dev-accounts/main.rs Outdated Show resolved Hide resolved
@evilrobot-01
Copy link
Collaborator

Just thinking about this a little, would it not make sense to add this as a command to the pop-node binary? e.g. pop-node init.

It then becomes a first-class citizen, usable whenever a network launched, rather than compiling another side-car executable and manually launching. It could remain as its own crate, e.g. pop-node-init and be wrapped in a default feature so that it can be compiled out for releases if necessary.

I'm just thinking about the UX via pop-cli, where currently the node is automatically sourced and built. A node would be launched, but would not have any funds and users would not be able to deploy any contracts.

Also - have the .scale files been optimised to only include metadata for the pallets which are actually used? They seem large and I am sure that there is an option to trim them done to only the functionality which is used.

scripts/fund-dev-accounts/main.rs Outdated Show resolved Hide resolved
scripts/fund-dev-accounts/main.rs Outdated Show resolved Hide resolved
@peterwht
Copy link
Collaborator Author

Also - have the .scale files been optimised to only include metadata for the pallets which are actually used? They seem large and I am sure that there is an option to trim them done to only the functionality which is used.

Unfortunately, it seems subxt has a bug with trimming metadata files (or I'm doing something wrong). Please see issue: paritytech/subxt#1509

@peterwht
Copy link
Collaborator Author

Just thinking about this a little, would it not make sense to add this as a command to the pop-node binary? e.g. pop-node init.

Love the idea. This will be done in a separate PR as time allows. See tracker #57

@peterwht peterwht requested review from al3mart and Daanvdplas March 28, 2024 18:08
Copy link
Collaborator

@evilrobot-01 evilrobot-01 left a comment

Choose a reason for hiding this comment

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

Seems the pop interface is missing?

Cargo.toml Outdated Show resolved Hide resolved
scripts/fund-dev-accounts/Cargo.toml Show resolved Hide resolved
scripts/fund-dev-accounts/Cargo.toml Outdated Show resolved Hide resolved
scripts/fund-dev-accounts/main.rs Outdated Show resolved Hide resolved
scripts/fund-dev-accounts/main.rs Outdated Show resolved Hide resolved
scripts/fund-dev-accounts/main.rs Outdated Show resolved Hide resolved
@peterwht peterwht requested a review from evilrobot-01 March 28, 2024 21:44
@Daanvdplas
Copy link
Collaborator

Everything works! Got an error messageQueue.ProcessingFailed but I think that was due xcmPallet.VersionNotifyRequested. The accounts were funded and the local network is set up as it should. One nitpick is the output:

Screenshot 2024-04-01 at 12 45 27

It would be nice to make this look plain and simple so that the developer can simply read what happened and doesn't get confused. E.g.:

XCM Reserved Transfer sent from relay.
Checking Pop Balances:
Alice: amount
Bob: amount
Charlie: amount

Another thing is documentation. We should explain simple and short what is happening when someone spins up a local network with Pop Network.

Both docs & output can be done in issue #57 as well

Copy link
Collaborator

@evilrobot-01 evilrobot-01 left a comment

Choose a reason for hiding this comment

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

LGTM. Looking forward to seeing it as a pop-node command!

@evilrobot-01
Copy link
Collaborator

Everything works! Got an error messageQueue.ProcessingFailed but I think that was due xcmPallet.VersionNotifyRequested. The accounts were funded and the local network is set up as it should. One nitpick is the output:

Interesting that the output includes Rococo, which should not be the case?

@Daanvdplas Daanvdplas merged commit 5708c0d into main Apr 2, 2024
5 checks passed
@Daanvdplas Daanvdplas deleted the peter/test-fund-script branch April 2, 2024 01:23
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.

4 participants