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

feat: add SRC-16 Typed Structured Data #161

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

Conversation

CatspersCoffee
Copy link

@CatspersCoffee CatspersCoffee commented Dec 4, 2024

Type of change

  • New Standard (SRC16)
  • Documentation
  • Example Implementations.

Changes

The following changes have been made:

  • Added SRC-16 standard for typed structured data hashing and signing.
  • Implemented core encoding functions for atomic, dynamic and reference types.
  • Added domain separator logic for cross-protocol signature safety.
  • Created comprehensive test suite for encoders (see: tests ).
  • Added example implementation with Mail struct for both SRC16Domain (Fuel) and EIP712Domain (Ethereum).
  • Included documentation on backwards compatability.

Notes

  • This standard enhances the Fuel ecosystem by providing a consistent way to handle structured data signing.
  • Compatible with existing EVM wallet infrastructure through domain type flexibility.
  • Designed to be extensible for future data types.
  • Test coverage includes both single value, dynamic array and fixed array encoding.

Related Issues

Closes #

Checklist

  • I have linked to any relevant issues.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have updated the documentation where relevant (API docs, the reference, and the Sway book).
    • If my change requires substantial documentation changes, I have requested support from the DevRel team
  • I have added tests that prove my fix is effective or that my feature works.
  • I have added the necessary New Feature labels where relevant.
  • I have done my best to ensure that my PR adheres to the Fuel Labs Code Review Standards.
  • I have requested a review from the relevant team or maintainers.
  • I have updated the changelog to reflect the changes on this PR.

@CatspersCoffee CatspersCoffee requested a review from a team as a code owner December 4, 2024 13:08
@fuel-cla-bot
Copy link

fuel-cla-bot bot commented Dec 4, 2024

Thanks for the contribution! Unfortunately we can't verify the commit author(s): Antony <b***@g***.com>. One possible solution is to add that email to your GitHub account. Alternatively you can change your commits to another email and force push the change. After getting your commits associated with your GitHub account, sign the Fuel Labs Contributor License Agreement and this Pull Request will be revalidated.

@fuel-cla-bot
Copy link

fuel-cla-bot bot commented Dec 4, 2024

Thanks for the contribution! Before we can merge this, we need @CatspersCoffee to sign the Fuel Labs Contributor License Agreement.

@SwayStar123
Copy link
Member

Just giving you a heads up, we will be reviewing this standard in the following days, but Fuel is enacting a code freeze around christmas as many contributors will be on vacation etc. So this will likely not be merged until January as the earliest.

Copy link
Member

@bitzoic bitzoic left a comment

Choose a reason for hiding this comment

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

This is amazing!

Just a few questions, primarily surrounding whether the existing encoder and hashing functionality in the std-lib and core are sufficent/can be integrated or if we need to introduce new traits and structs.

I'm also curious on how handling Fuel specific types would work. We've made the distinct choice to separate the Address and ContractId types as well as have AssetId.


Atomic Types:
```sway
bytes1 to bytes32
Copy link
Member

Choose a reason for hiding this comment

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

What would this look like in practice? Sway does not have a bytes1 type

Copy link
Author

Choose a reason for hiding this comment

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

Thankyou for pointing out this mistake. This has been corrected to:

Atomic Types:

u8 to u256
bool
b256 (hash)

Dynamic Types:

Bytes   // Variable-length byte sequences
String  // Variable-length strings

Reference Types:

Arrays (both fixed size and dynamic)
Structs (reference to other struct types)

/// A demo struct representing a mail message
pub struct Mail {
/// The sender's address
pub from: b256,
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
pub from: b256,
pub from: Address,
Suggested change
pub from: b256,
pub from: ContractId,
Suggested change
pub from: b256,
pub from: Identity,

To follow Sway principles, this should use either Address, ContractId, or Identity.
The same applies to to below.

Copy link
Author

Choose a reason for hiding this comment

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

This has been adjusted in the Fuel example to reflect the Address native type.

/// A demo struct representing a mail message
pub struct Mail {
    /// The sender's address
    pub from: Address,
    /// The recipient's address
    pub to: Address,
    /// The message contents
    pub contents: String,
}

/// * [b256] - The Keccak256 hash of the encoded domain parameters
///
pub fn domain_hash(self) -> b256 {
let mut encoded = Bytes::new();
Copy link
Member

Choose a reason for hiding this comment

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

This looks like it could use the AbiEncode trait from core https://fuellabs.github.io/sway/master/core/codec/index.html

Copy link
Author

Choose a reason for hiding this comment

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

Thankyou for the suggestion. I have added:

impl AbiEncode for SRC16Domain { ... }

and

impl AbiEncode for EIP712Domain { ... }


/// Trait for types that can be hashed in a structured way
///
pub trait TypedDataHash {
Copy link
Member

Choose a reason for hiding this comment

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

Do we need a new trait or will Hash work here? I believe the implementations are the same. https://fuellabs.github.io/sway/master/std/hash/trait.Hash.html

Copy link
Author

Choose a reason for hiding this comment

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

This trait is required as the implementer needs to write (or generate) TypedDataHash for <Their_Struct_Type> in line with the definition of hashStruct.

The TypedDataHash is not a simple Hasher, but an encoding scheme and final hash.

standards/src/src16.sw Outdated Show resolved Hide resolved
examples/src16-typed-data/fuel_example/src/main.sw Outdated Show resolved Hide resolved
@bitzoic bitzoic requested a review from a team December 11, 2024 12:35
@bitzoic bitzoic added the New Standard Label used to filter for the introduction of a new standard label Dec 11, 2024
@bitzoic bitzoic added the SRC-16 Label used to filter for the standard issue label Dec 11, 2024
docs/src/src-16-typed-structured-data.md Outdated Show resolved Hide resolved
docs/src/src-16-typed-structured-data.md Show resolved Hide resolved
docs/src/src-16-typed-structured-data.md Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla:signed New Standard Label used to filter for the introduction of a new standard SRC-16 Label used to filter for the standard issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants