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

pedrouid
Copy link
Contributor

@pedrouid pedrouid commented Jul 10, 2019

Fix #3

@pedrouid
Copy link
Contributor Author

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
Copy link
Contributor Author

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
Copy link
Contributor Author

pedrouid 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.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe better as:

from the address you provided

const BigNumber = window.BigNumber;

const TOKEN_TRANSFER = "0xa9059cbb";
const DAI_TOKEN_ADDRESS = "0x89d24A6b4CcB1B6fAA2625fE562bDD9a23260359";
Copy link
Collaborator

Choose a reason for hiding this comment

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

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 }
Copy link
Collaborator

Choose a reason for hiding this comment

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

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(
Copy link
Collaborator

Choose a reason for hiding this comment

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

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()
Copy link
Collaborator

Choose a reason for hiding this comment

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

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.

@pedrouid pedrouid mentioned this pull request Jul 10, 2019
@pedrouid
Copy link
Contributor Author

pedrouid 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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support WalletConnect
3 participants