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.

Sign up for GitHub

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

Jump to bottom

update TransferIntent id type to bytes32 #18

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

update TransferIntent id type to bytes32 #18

wants to merge 1 commit into from

Conversation

kylekaplan
Copy link

Hi,

I was wondering if the id field of the TransferIntent could be updated to a bytes32 instead of bytes16.

This would make it so the Keccak256 hash function could be used to generate predictable ids and be more compatible with other protocols generating ids with Keccak256.

For my use case, I'm using EAS (Ethereum Attestation Service) as a backend to create an on-chain eCommerce solution, and it would be nice if I could use an attestation UID (a Keccak256 hash of the attestation) as the id for the TransferIntent. This would make the relationship between an attestation and a TransferIntent simpler and more efficient.


Thanks for your consideration and for this awesome protocol.

@cb-heimdall
Copy link

🟡 Heimdall Review Status

Requirement Status More Info
Reviews 🟡 0/1
Denominator calculation
Show calculation
1 if user is bot 0
1 if user is external 0
From .codeflow.yml 1
Additional review requirements
Show calculation
Max 0
0
From CODEOWNERS 0
Global minimum 0
Max 1
1
1 if commit is unverified 1
Sum 2

@Nichebiche
Copy link

Agreed

TELLYWHALE
TELLYWHALE approved these changes Aug 11, 2024
View reviewed changes
Copy link

@TELLYWHALE TELLYWHALE left a comment •
edited
Loading

Choose a reason for hiding this comment

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

🐳

@cb-heimdall
Copy link

Review Error for TELLYWHALE @ 2024-08-11 00:05:58 UTC
User must have write permissions to review

TELLYWHALE
TELLYWHALE approved these changes Aug 11, 2024
View reviewed changes
Copy link

@TELLYWHALE TELLYWHALE left a comment

Choose a reason for hiding this comment

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

🤞🏾

@cb-heimdall
Copy link

Review Error for TELLYWHALE @ 2024-08-11 00:30:59 UTC
User must have write permissions to review

Nichebiche
Nichebiche approved these changes Aug 27, 2024
View reviewed changes
Copy link

@Nichebiche Nichebiche left a comment

Choose a reason for hiding this comment

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

Permissions

@cb-heimdall
Copy link

Review Error for Nichebiche @ 2024-08-27 21:09:33 UTC
User must have write permissions to review

Nichebiche
Nichebiche reviewed Aug 27, 2024
View reviewed changes
Copy link

@Nichebiche Nichebiche left a comment

Choose a reason for hiding this comment

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

Requesting write access

@cb-heimdall
Copy link

Review Error for MTerrases22 @ 2024-09-21 22:09:21 UTC
User must have write permissions to review

@Nichebiche
Copy link

Nichebiche commented Sep 21, 2024 via email

Nichebiche
Nichebiche approved these changes Sep 21, 2024
View reviewed changes
Copy link

@Nichebiche Nichebiche left a comment

Choose a reason for hiding this comment

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

I hope this means I have write access now. Just learning over here:)

@cb-heimdall
Copy link

Review Error for Nichebiche @ 2024-09-21 22:14:21 UTC
User must have write permissions to review

@cb-heimdall
Copy link

Review Error for Irasone @ 2024-09-21 22:43:13 UTC
User must have write permissions to review

@cb-heimdall
Copy link

Review Error for Irasone @ 2024-09-21 22:44:29 UTC
User must have write permissions to review

@MTerrases22
Copy link

Why haven't I received payment? I've been mistreated here. This is my account and no one should be preventing me from it. I'm so not letting this one go . I can't retrieve my funds. Someone took my identity and keeps locking me out of my account. I want my account left alone. It's my money not anyone else's.

Sturgeontara06
Sturgeontara06 reviewed Oct 11, 2024
View reviewed changes
Copy link

@Sturgeontara06 Sturgeontara06 left a comment

Choose a reason for hiding this comment

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

Sure

@MTerrases22
Copy link

MTerrases22 commented Oct 11, 2024 via email

@cb-heimdall
Copy link

Review Error for Irasone @ 2024-10-11 01:46:04 UTC
User must have write permissions to review

@MTerrases22
Copy link

MTerrases22 commented Oct 12, 2024 via email

@MTerrases22
Copy link

MTerrases22 commented Oct 12, 2024 via email

@MTerrases22
Copy link

How can I access that check. With authorization of course. Been locked out and unable to claim my assets.someone please help me.

@MTerrases22
Copy link

MTerrases22 commented Oct 20, 2024 via email

@cb-heimdall
Copy link

Review Error for Tinaochoa8462 @ 2024-10-24 07:38:24 UTC
User must have write permissions to review

@cb-heimdall
Copy link

Review Error for joeltato @ 2024-11-07 11:33:52 UTC
User must have write permissions to review

@joeltato
Copy link

joeltato commented Nov 7, 2024 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Sturgeontara06 Sturgeontara06 Sturgeontara06 left review comments

@joeltato joeltato joeltato approved these changes

@MTerrases22 MTerrases22 MTerrases22 approved these changes

@Irasone Irasone Irasone approved these changes

@Nichebiche Nichebiche Nichebiche approved these changes

@TELLYWHALE TELLYWHALE TELLYWHALE approved these changes

@Tinaochoa8462 Tinaochoa8462 Tinaochoa8462 approved these changes

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
9 participants
@kylekaplan @cb-heimdall @Nichebiche @MTerrases22 @joeltato @Tinaochoa8462 @TELLYWHALE @Sturgeontara06 @Irasone