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

Gasless Asks Extension #160

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Gasless Asks Extension #160

wants to merge 5 commits into from

Conversation

tbtstl
Copy link
Contributor

@tbtstl tbtstl commented Apr 3, 2022

New Module Extension Proposal

Description

This extension enables signed off-chain orders to be executed on the Asks Core ETH module.

Motivation and Context

In order to support gas-free NFT platforms, we can extend our Asks module to support signed orders as listings. We can also use this extension as a mechanism to drive on-chain liquidity via incentive programs. This extension provides support for the execution, invalidation, broadcasting, and promoting of off-chain signed orders via EIP-712.

How has this been tested?

Relevant tests have been included via Foundry.

Checklist:

  • The module includes tests written for Foundry
  • The module is documented with NATSPEC
  • The documentation includes UML Diagrams for external and public functions
  • The module is a Hyperstructure

Comment on lines 7 to 9
uint8 v; // The 129th byte and chain ID of the signature
bytes32 r; // The first 64 bytes of the signature
bytes32 s; // Bytes 64-128 of the signature
Copy link

Choose a reason for hiding this comment

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

Should these comments instead say 65th byte, first 32 bytes, and bytes 32-64, respectively? Or am I missing another part of the signature?

Copy link

Choose a reason for hiding this comment

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

Lmao I honestly didn't even expect to have review permission on this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it's correct – from the EIP-712 spec:

As in eth_sign it is a hex encoded 129 byte array starting with 0x. It encodes the r, s and v parameters from appendix F of the yellow paper in big-endian format. Bytes 0…64 contain the r parameter, bytes 64…128 the s parameter and the last byte the v parameter. Note that the v parameter includes the chain id as specified in EIP-155.

Copy link

Choose a reason for hiding this comment

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

Ohh yea the ERC is very unclear with the terminology there. Seems it confuses bytes with hex characters in that paragraph (the appendix it references also says there are 65 bytes in a sig). Thanks for linking!

Copy link

@aunyks aunyks Apr 3, 2022

Choose a reason for hiding this comment

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

Lol it also said to expect a 129 byte array as a signature but shows a 65 byte array in the example just below?? I’m gonna try and open a PR over in ethereum/EIPs

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.

3 participants