paritytech / polkadot-testnet-faucet

https://faucet.polkadot.io/
MIT License
36 stars 34 forks source link

Integrate ETH address conversion; ensure parachain links #459

Closed KarimJedda closed 1 week ago

KarimJedda commented 2 weeks ago

Quoting from @pgherveou :

it would be nice if we could update https://faucet.polkadot.io/westend with the followings:

  • Have a custom url for each chain, instead of asking the user to pick a chain from the dropdown, it's too easy to forget to do it and send the fund to the relay chain instead.
  • Detect 0x Eth address so that you can fund directly Eth address
lovelaced commented 2 weeks ago

@pgherveou how is the eth address mapped to a substrate address?

pgherveou commented 2 weeks ago

@smiasojed , I think you already did something similar for Remix, do you have some js code to go from Eth -> AccountId32 that we can reuse here?

smiasojed commented 2 weeks ago

Yes, I did, code: https://github.com/paritytech/revive-remix/blob/2eca8b351f96734ffc94adc63b94f3587bd8d67d/libs/remix-ui/helper/src/lib/remix-ui-helper.ts#L93

pgherveou commented 2 weeks ago

@lovelaced let us know if you need help on this. how long do you think it would take to make / publish these changes?

mutantcornholio commented 2 weeks ago

Doesn't look complex. I'll try to get it to PR stage today.
Should address conversion work on any testnet, or only on Westend?

lovelaced commented 2 weeks ago

@mutantcornholio only required for westend asset hub for now, but if it's easier to add for all chains that's also fine

mutantcornholio commented 2 weeks ago

@mutantcornholio only required for westend asset hub for now, but if it's easier to add for all chains that's also fine

Adding it to all chains would be easier. Will it work as expected?

lovelaced commented 2 weeks ago

@pgherveou @smiasojed do you have any input on the above? ^

athei commented 2 weeks ago

It works the same for all chains: 1 ) Take the 20 bit eth address. 2) Suffix with 12 bytes of 0xEE in order to get 32byte address 3) SS58 encode

smiasojed commented 2 weeks ago

For Kusama or Polkadot will be different prefix in SS58 encode.

mutantcornholio commented 2 weeks ago

It works the same for all chains:...

Ah yeah, thanks, the pieces came together now.

For Kusama or Polkadot will be different prefix in SS58 encode.

There's no faucet for Kusama / Polkadot, but good to keep in mind. I'll probably make the prefix configurable.

mutantcornholio commented 2 weeks ago
  • Have a custom url for each chain, instead of asking the user to pick a chain from the dropdown, it's too easy to forget to do it and send the fund to the relay chain instead.

Turns out, it's actually already implemented: https://faucet.polkadot.io/westend?parachain=1000
What's not implemented, however, is updating the address when changing the parachain from the dropdown 🤦‍♂️. Will fix.

pgherveou commented 2 weeks ago
  • Have a custom url for each chain, instead of asking the user to pick a chain from the dropdown, it's too easy to forget to do it and send the fund to the relay chain instead.

Turns out, it's actually already implemented: https://faucet.polkadot.io/westend?parachain=1000 What's not implemented, however, is updating the address when changing the parachain from the dropdown 🤦‍♂️. Will fix.

seems to work for me, it does update it to assethub

mutantcornholio commented 2 weeks ago

seems to work for me, it does update it to assethub

I meant the other way around: select a different parachain from the dropdown → the url doesn't get updated.
You can send direct link to people, but you've got no way to get direct link, without prior knowledge.

pgherveou commented 2 weeks ago

ah right that's correct, I think it will still be nice if not too much extra works to have a "clean url" without querystring

mutantcornholio commented 2 weeks ago

ah right that's correct, I think it will still be nice if not too much extra works to have a "clean url" without querystring

What's wrong with querystring?

pgherveou commented 2 weeks ago

I get it's ok this is just bike shedding, we need to reference the faucets on some repo, .../asset-hub just looks nicer but we will go with parachain=1000 for now

Quick question can you also specify the address from the querystring?

mutantcornholio commented 2 weeks ago

Quick question can you also specify the address from the querystring?

Yeah, but could you explain the usecase for this?

pgherveou commented 2 weeks ago

use case would be to create link that prefill everything, once you have connected your wallet, we can point you to the url where you can ask for test funds by prefilling all the details

lovelaced commented 2 weeks ago

@pgherveou do you mean we would be linking directly from remix or something? Where is the user going to be connecting their wallet and being redirected to the faucet? (just for context)

athei commented 2 weeks ago

We will be linking to the faucet from our tutorial: https://github.com/paritytech/contract-docs/pull/10

Would be nice if we can select the correct network via direct link. Otherwise the user has to select the correct network.

lovelaced commented 2 weeks ago

We will be linking to the faucet from our tutorial: paritytech/contract-docs#10

Would be nice if we can select the correct network via direct link. Otherwise the user has to select the correct network.

Yes, this is already possible. I was wondering about the context for prefilled addresses.

mutantcornholio commented 2 weeks ago

I'm trying to conjure a meaningful e2e test for address conversion.
If my goal is to obtain a usable eth address... Do I deploy a smart contract?

I want to avoid duplicating the same conversion logic both in code and in test

athei commented 2 weeks ago

We will be linking to the faucet from our tutorial: paritytech/contract-docs#10 Would be nice if we can select the correct network via direct link. Otherwise the user has to select the correct network.

Yes, this is already possible. I was wondering about the context for prefilled addresses.

Not sure what @pgherveou has in mind. But we could use it to fill the address that is selected in Metamask. Doesn't seem like a high priority, though.

pgherveou commented 2 weeks ago

yeah not a hight priority at all just checking if this could be done at all

lovelaced commented 2 weeks ago

I'm trying to conjure a meaningful e2e test for address conversion. If my goal is to obtain a usable eth address... Do I deploy a smart contract?

I want to avoid duplicating the same conversion logic both in code and in test

You can just do an ethereum transfer (you can generate a regular eth address in metamask, and then send funds to another address). You could probably use web3.js or ethers.js for this if that makes sense, you can view such transactions here with the resulting substrate addresses.

lovelaced commented 2 weeks ago

yeah not a hight priority at all just checking if this could be done at all

Yes it's definitely possible, but we can revisit this if there's a direct need for it

mutantcornholio commented 2 weeks ago

The changes were deployed, try using 0x.. address here: https://faucet.polkadot.io/westend?parachain=1000

lovelaced commented 2 weeks ago

Nice, worked for me @mutantcornholio ! 🎉

lovelaced commented 1 week ago

@mutantcornholio the RPC URL needs to be changed to https://westend-asset-hub-eth-rpc.polkadot.io

in addition, can the placeholder text in the address field say something like: "5rt6... or 0x833..."?

mutantcornholio commented 1 week ago

@mutantcornholio the RPC URL needs to be changed to https://westend-asset-hub-eth-rpc.polkadot.io

Why? Faucet dripper account lives on relay chain, and we're teleporting tokens to AH.

in addition, can the placeholder text in the address field say something like: "5rt6... or 0x833..."?

sure

lovelaced commented 1 week ago

Oh, sorry, my misunderstanding - I thought the eth RPC was used somehow here.

mutantcornholio commented 1 week ago

in addition, can the placeholder text in the address field say something like: "5rt6... or 0x833..."?

https://github.com/paritytech/polkadot-testnet-faucet/pull/461