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

Metamask & WalletConnect support #26

Closed
wants to merge 25 commits into from

Conversation

Projects
None yet
3 participants
@pedrouid
Copy link
Contributor

commented Jul 10, 2019

Fix #3

@pedrouid

This comment has been minimized.

Copy link
Contributor Author

commented Jul 10, 2019

Still need to work out some details about how to submit the Transaction Hash to the Pretix order

@pedrouid pedrouid changed the title WalletConnect support Metamask & WalletConnect support Jul 10, 2019

pedrouid added some commits Jul 10, 2019

@pedrouid

This comment has been minimized.

Copy link
Contributor Author

commented Jul 10, 2019

I've spent some time resolving some merge conflicts with master. Still need to think about the flow for paying with Metamask and WalletConnect.

Previously the plugin would request the user to input their sender address and use that to monitor for outgoing transactions that matched the receiving address

Now we have moved this logic until after the order is completed. The user only needs to select the currency which wants to pay with (either ETH or DAI) then after finalizing the order, it gets to the pending.html view. At this point we have calculated the price rate of the selected currency that needs to be paid for and we prompt the user to connect with Metamask or WalletConnect.

Right after successful connection we make a transaction requests with the provided amount calculated from _get_rates passed down through payment_pending_render. Once the user signs the transaction and broadcasts we get a transaction hash that can be verified on chain using the logic at txn_check.

There is still one issue. We didn't know how to pass the transaction hash from the client-side to the pretix plugin on the backend programatically. Should it be made into a separate form? Is there an API endpoint that could be used to update the order? You can find the client-side logic for the transaction request . on connect.js.

Please let me know if this was clear, I would like to help get this merged asap

@pedrouid

This comment has been minimized.

Copy link
Contributor Author

commented Jul 10, 2019

Maybe a solution would be to move the connect.js logic of this PR to the checkout_payment_confirm.html view on the payment section which you can see below.
Screenshot 2019-07-10 15 48 54
At this point we already had the currency selection by the user on the previous view (checkout_payment_form.html) thus we can calculate the required amount. Then we can use the submit button labeled as Place binding order with the provided txn_hash from Metamask/WalletConnect. This way we can display on the pending.html the status of the transaction.

@@ -2,6 +2,6 @@

<p>{% blocktrans trimmed %}
After you submitted your order, we will provide you with {{ currency }} address to deposit an equivalent ethereum amount.
Please note that we will only give a ticket provided you deposit from {{ from }}.
Please note that we will only give a ticket provided you deposit from your address you provided.

This comment has been minimized.

Copy link
@pipermerriam

pipermerriam Jul 10, 2019

Collaborator

Maybe better as:

from the address you provided

const BigNumber = window.BigNumber;

const TOKEN_TRANSFER = "0xa9059cbb";
const DAI_TOKEN_ADDRESS = "0x89d24A6b4CcB1B6fAA2625fE562bDD9a23260359";

This comment has been minimized.

Copy link
@pipermerriam

pipermerriam Jul 10, 2019

Collaborator

Am I correct that this moves us from the POA DAI network to the one on mainnet (no problems with thus, just want to be sure I understand).


def json_rpc_request(method, params):
try:
payload = { "id": 1337, "jsonrpc": "2.0", "method": method, "params": params }

This comment has been minimized.

Copy link
@pipermerriam

pipermerriam Jul 10, 2019

Collaborator

I'll come through and cleanup this file with a pattern that lets us test it in a follow up PR so assuming this code works right now I don't see any need to do much cleanup on it since I'll rework most of it.

return bool(
(self.settings.DAI or self.settings.ETH) and super().is_allowed(request)
)
return bool(

This comment has been minimized.

Copy link
@pipermerriam

pipermerriam Jul 10, 2019

Collaborator

These lines need to indented one more level.

return None

def check_txn_confirmation(txn_hash, from_address, to_address, currency, amount, timestamp):
from_address = from_addres.lower()

This comment has been minimized.

Copy link
@pipermerriam

pipermerriam Jul 10, 2019

Collaborator

Check this failing CI run: https://circleci.com/gh/esPass/pretix-eth-payment-plugin/22

This file currently has a number of simple errors like mispelled variable names.

jinchung and others added some commits Jul 10, 2019

fix

pedrouid added some commits Jul 10, 2019

@pedrouid pedrouid referenced this pull request Jul 10, 2019

Closed

Insert Transaction Hash #32

@pedrouid

This comment has been minimized.

Copy link
Contributor Author

commented Jul 10, 2019

Replaced by #32

@pedrouid pedrouid closed this Jul 10, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.