Skip to content

CEP-2612: Permit Extension#99

Open
zie1ony wants to merge 2 commits into
casper-network:masterfrom
odradev:cep2612
Open

CEP-2612: Permit Extension#99
zie1ony wants to merge 2 commits into
casper-network:masterfrom
odradev:cep2612

Conversation

@zie1ony
Copy link
Copy Markdown
Contributor

@zie1ony zie1ony commented May 12, 2026

Copy link
Copy Markdown

@davidatwhiletrue davidatwhiletrue left a comment

Choose a reason for hiding this comment

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

@zie1ony , please check my comments.

Comment thread text/2612-permit-extension.md
Comment thread text/2612-permit-extension.md Outdated

The CEP-2612 extension is defined by:
- one new entrypoint (`permit`),
- the EIP-712 typed data definition for the `Permit` struct,
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

'...and the domain separator'

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done

Comment thread text/2612-permit-extension.md Outdated

Below definitions use Rust syntax, but they are not Rust specific.

### Entrypoint interface
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

'Entry point' with a space

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done

Comment thread text/2612-permit-extension.md Outdated
/// disables the expiry check.
/// - `public_key` is the signer's Casper public key.
/// - `signature` is `public_key`'s signature over the EIP-712 digest
/// of the `Permit` typed data (see below).
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I'd remove (see below) becuase this is a code block.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done

Comment thread text/2612-permit-extension.md
Comment thread text/2612-permit-extension.md Outdated

1. `signature` is a valid signature of `public_key` over the EIP-712
digest of the `Permit` typed data (described below), AND
2. `owner == Address::from(public_key)` — i.e. `owner` is the account
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Is Address::from() correct here?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes. It only represents the logic.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I'm not convinced. Address is an Odra type. It doesn't exist in casper_types. The reader doesn't need to know that an Address is an account hash. It'd be more explicit.

Comment thread text/2612-permit-extension.md Outdated
The encoded message data hashed alongside the typehash is the
concatenation, in order, of:

- `owner` encoded as an EIP-712 `address` (32 bytes, big-endian, left-padded),
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

In Casper, owner and spender are a hash of an Account Hash or a (package) Hash (the hash of 0x00 or 0x01 prefix || 32 bytes). I would mention it here. and remove big-endian, left-padded

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done, I mentioned how address is serialized.

Comment thread text/2612-permit-extension.md Outdated
- `nonce` encoded as an EIP-712 `uint256` (32 bytes, big-endian) — equal
to the current on-chain `permit_nonces[owner]` value at the time the
signature was produced,
- `deadline` encoded as an EIP-712 `uint64` (32 bytes, big-endian,
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

“EIP-712 uint64" does not exist. u64 or uint256. let’s use the same here as in CEP-3009 for validAfter and validBefore

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done

Comment thread text/2612-permit-extension.md Outdated
The final digest is computed using the standard EIP-712 rule
(`keccak256("\x19\x01" || domainSeparator || keccak256(typeHash || encodedData))`).

The EIP-712 `domainSeparator` uses:
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Not aligned with casper-eip-712 domain type:

EIP712Domain(string name,string version,string chain_name,bytes32 contract_package_hash).

Worth to add the type string to this section too.

Also, recommend to use CAIP-2 for chain_name as here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I updated the document, but not sure if this is what you wanted. Please check

Copy link
Copy Markdown

@davidatwhiletrue davidatwhiletrue May 20, 2026

Choose a reason for hiding this comment

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

I don't see the domain type in the document. Because it differs from the EIP-2612 domain type, I think we need to include it.

EIP712Domain(string name,string version,string chain_name,bytes32 contract_package_hash)

Also, check this comment: https://github.com/casper-network/ceps/pull/99/changes#r3272351456

Comment thread text/2612-permit-extension.md
Copy link
Copy Markdown

@davidatwhiletrue davidatwhiletrue left a comment

Choose a reason for hiding this comment

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

@zie1ony , see my comments.

The final digest is computed using the standard EIP-712 rule
(`keccak256("\x19\x01" || domainSeparator || keccak256(typeHash || encodedData))`).

The `domainSeparator` is defined as:
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This is not the domain separator for casper.
See here:
https://github.com/casper-ecosystem/casper-eip-712#casper-native-domain-example

EIP712Domain(string name,string version,string chain_name,bytes32 contract_package_hash)

Comment thread text/2612-permit-extension.md Outdated
The final digest is computed using the standard EIP-712 rule
(`keccak256("\x19\x01" || domainSeparator || keccak256(typeHash || encodedData))`).

The EIP-712 `domainSeparator` uses:
Copy link
Copy Markdown

@davidatwhiletrue davidatwhiletrue May 20, 2026

Choose a reason for hiding this comment

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

I don't see the domain type in the document. Because it differs from the EIP-2612 domain type, I think we need to include it.

EIP712Domain(string name,string version,string chain_name,bytes32 contract_package_hash)

Also, check this comment: https://github.com/casper-network/ceps/pull/99/changes#r3272351456


`Address` encoding is defined as 33 bytes, where the first byte is a type tag:
- `0x00` for `AccountHash`,
- `0x01` for `PackageHash`.
Copy link
Copy Markdown

@davidatwhiletrue davidatwhiletrue May 20, 2026

Choose a reason for hiding this comment

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

Hash instead of PackageHash?
o "0x01 for a package Hash"

@mssteuer mssteuer self-requested a review May 22, 2026 00:46
@mssteuer
Copy link
Copy Markdown
Member

@zie1ony once @davidatwhiletrue's remaining comments are addressed/responded to, we can finalize this

- `0x01` for `PackageHash`.

The encoded message data hashed alongside the typehash is the
concatenation of `CLValue` representations, in order, of:
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I would simplify this sentence to:
"The encodeData value is the concatenation of the following EIP-712-encoded fields, in order:"

'the concatenation of CLValue representations' is not correct.

I'd use uint256 instead of U256.

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.

3 participants