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

Add ERC-7798: Tap to Pay #686

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open

Add ERC-7798: Tap to Pay #686

wants to merge 10 commits into from

Conversation

amhed
Copy link

@amhed amhed commented Oct 26, 2024

See https://hackmd.io/VyxIMlk1SvCOpBpS6a_2uA?both for initial commentary on the standard

@eip-review-bot
Copy link
Collaborator

eip-review-bot commented Oct 26, 2024

File ERCS/erc-7798.md

Requires 1 more reviewers from @axic, @g11tech, @SamWilsn, @xinbenlv

@eip-review-bot eip-review-bot changed the title ERC Proposal: Tap to pay open standard Add ERC: Contactless Crypto Payment Standard Oct 26, 2024
@github-actions github-actions bot added the w-ci label Oct 26, 2024
ERCS/erc-_____.md Outdated Show resolved Hide resolved
1. The merchant is running an app on their point-of-sale (PoS) system that allows initiating a checkout flow.
1. When the customer is done ordering, the merchant goes through the checkout flow up and triggers a payment request.
1. The merchant app calls the `requestContactlessPayment` RPC method with the expected payload.
1. The merchant's device emits an NFC signal via host card emulation ([HCE](https://developer.android.com/develop/connectivity/nfc/hce)) for the customer’s device to read.

Choose a reason for hiding this comment

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

Is this done via the Ethereum provider receiving the above call? So is it assumed the provider needs to be running on device?

Copy link
Author

Choose a reason for hiding this comment

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

Yes

1. When the customer is done ordering, the merchant goes through the checkout flow up and triggers a payment request.
1. The merchant app calls the `requestContactlessPayment` RPC method with the expected payload.
1. The merchant's device emits an NFC signal via host card emulation ([HCE](https://developer.android.com/develop/connectivity/nfc/hce)) for the customer’s device to read.
1. The customer, running their wallet software of choice, reads the NFC signal. The customer’s wallet constructs the necessary transaction(s) to execute the onchain checkout.

Choose a reason for hiding this comment

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

Sorry I don't know much about this, why couldn't we just do a normal eth_sendTransaction or wallet_sendCalls via an NFC channel?

Copy link
Author

Choose a reason for hiding this comment

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

We had some sluggish performance transmitting the NFC message with larger payloads and decided to use a relayer for a shorter URI.

Copy link
Contributor

Choose a reason for hiding this comment

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

you could still use a relayer by using wallet_sendCalls + something like a nfcRelay capability

1. Upon successful onchain execution of these transactions, the checkout is complete.

#### Use Case 2: Peer-to-peer Sends
1. Alice requests a payment on her wallet app by emitting an NFC signal. The contents of the signal is an EIP-681 URI

Choose a reason for hiding this comment

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

I think 681 is possibly over specified on things like gas values. Wonder if we could align on something more minimal.

Copy link
Author

Choose a reason for hiding this comment

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

We're already using 681 heavily on send/receive flows on CB Wallet. We tried leaning into existing standards. What would you suggest could be better?

- 1 means the URI is an EIP-681-compliant payload.
- 2 means the URI points to an HTTP relayer, where the response of the relayer is either a JSON object with the transaction data already encoded as a 0x string, or a message for the customer to sign. The customer wallet is responsible for constructing, signing and submitting the transaction/message.

- `uri` represents either the EIP-681-compliant payload or the relayer endpoint to query for the JSON payload.

Choose a reason for hiding this comment

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

I feel like 681 URI is not widely adopted and is mostly useful when you can share a string with someone and not a whole payload.

But here we control the channel so I'm again wondering why not just use existing wallet RPCs to describe what we want.

Copy link
Author

Choose a reason for hiding this comment

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

You're right, it's not widely adopted. Open to any other suggestions 🙏

@amhed
Copy link
Author

amhed commented Oct 26, 2024

Thanks for the quick review @wilsoncusack ❤️

ERCS/erc-_____.md Outdated Show resolved Hide resolved
ERCS/erc-_____.md Outdated Show resolved Hide resolved
ERCS/erc-_____.md Outdated Show resolved Hide resolved
Co-authored-by: Andrew B Coathup <28278242+abcoathup@users.noreply.github.com>
@eip-review-bot eip-review-bot changed the title Add ERC: Contactless Crypto Payment Standard Add ERC: Contactless Payment Oct 28, 2024
ERCS/erc-_____.md Outdated Show resolved Hide resolved
Co-authored-by: Andrew B Coathup <28278242+abcoathup@users.noreply.github.com>
@github-actions github-actions bot added w-ci and removed w-ci labels Oct 29, 2024
ERCS/erc-7798.md Outdated
```

##### EIP-712
```ts
Copy link
Author

Choose a reason for hiding this comment

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

I don't think 712 is available on this repository. What's the proper way to do a relative ref here?

@github-actions github-actions bot removed the w-ci label Oct 29, 2024
- Some fixes on automated PR feedback
- Remove relative links
- Adds copyright waiver
@eip-review-bot eip-review-bot changed the title Add ERC: Contactless Payment Add ERC: Tap to Pay Oct 29, 2024
@github-actions github-actions bot added the w-ci label Oct 29, 2024
@amhed amhed changed the title Add ERC: Tap to Pay ERC-7798: Tap to Pay Oct 29, 2024
@github-actions github-actions bot added w-ci and removed w-ci labels Oct 29, 2024
@eip-review-bot eip-review-bot changed the title ERC-7798: Tap to Pay Add ERC: Tap to Pay Oct 30, 2024
});
```

### Type 1
Copy link
Contributor

Choose a reason for hiding this comment

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

are offchain signatures supported for type 1? i dont think it's covered by 681?

Copy link
Author

Choose a reason for hiding this comment

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

No, we covered them with the relayed URI. You can basically pass down any long-form payload / set of batched transactions for EIP-3009 or permit2

}
```

**[EIP-712](https://eips.ethereum.org/EIPS/eip-712) Message (offline signature)**
Copy link
Contributor

Choose a reason for hiding this comment

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

hm i guess sendCalls wouldnt work for getting offchain signatures

@lukasrosario
Copy link
Contributor

overall i agree with Wilson's feedback -- feel like it would be cleanest if we could just transmit the usual RPC request over the NFC channel. would be easier for devs to reason about and i think it would be easier to adopt than extending the 1193 provider.

seems like there were issues transmitting larger messages through nfc, and it makes me wonder if that should just be an issue for the provider to resolve if the message is to be transmitted over NFC. that way the app dev can use the same interface as usual, and then the wallet maintainers resolve how to compress & transmit to wallet.

@amhed
Copy link
Author

amhed commented Oct 30, 2024

seems like there were issues transmitting larger messages through nfc, and it makes me wonder if that should just be an issue for the provider to resolve if the message is to be transmitted over NFC. that way the app dev can use the same interface as usual, and then the wallet maintainers resolve how to compress & transmit to wallet.

When you mention the same interface you mend wallet_sendCalls? And send URIs instead of hashed strings representing txs?

@amhed amhed changed the title Add ERC: Tap to Pay Add ERC-7798]: Tap to Pay Nov 5, 2024
@amhed amhed changed the title Add ERC-7798]: Tap to Pay Add ERC-7798: Tap to Pay Nov 5, 2024
Copy link

github-actions bot commented Nov 5, 2024

The commit b0b33c8 (as a parent of 097acbf) contains errors.
Please inspect the Run Summary for details.

@lukasrosario
Copy link
Contributor

When you mention the same interface you mend wallet_sendCalls?
correct

And send URIs instead of hashed strings representing txs?
i dont have as strong of an opinion on how the transaction payloads themselves are structured. in fact, if we keep the developer interface the same and just use a 5792 capability, for example, then we don't even need to impose any kind of constraint on the transaction payload format and can let the provider choose the format as they wish. i suppose the problem then becomes you lose the ability to do offchain signatures. i wonder how much of an issue that would be though

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants