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

refactor: Update the TxsHash + add TxEffect #3938

Closed
Tracked by #3533
, #4492
...
benesjan opened this issue Jan 11, 2024 · 1 comment · Fixed by #4726
Closed
Tracked by #3533
, #4492
...

refactor: Update the TxsHash + add TxEffect #3938

benesjan opened this issue Jan 11, 2024 · 1 comment · Fixed by #4726
Assignees
@benesjan
Milestone
Execution Environ...

Comments

@benesjan
Copy link
Contributor

benesjan commented Jan 11, 2024 •
edited by LHerskind
Loading

Look for TODO(#3938) in the codebase.

Note: Nuke EthAddress.toBuffer32() while updating the hashing --> EthAddress should make only 20 bytes of preimage.

The issue should have one main concern. To alter the BODY hashing, here meaning the TxEffects hashing to align, circuits/yellow-paper/typescript/l1.

I think most of it is already aligned, but making sure that this is up to date will be useful for #4492 where some of it will be altered to accommodate the changes as part of the message changes.
@benesjan benesjan mentioned this issue Jan 11, 2024
Closed
@just-mitch just-mitch mentioned this issue Jan 12, 2024
Merged
@benesjan benesjan changed the title Update the content hashing (circuits + l1contracts) + add TxEffect Update the body hashing (circuits + l1contracts) + add TxEffect Jan 17, 2024
@benesjan benesjan mentioned this issue Jan 17, 2024
Merged
@LHerskind LHerskind mentioned this issue Jan 18, 2024
Merged
@LHerskind LHerskind added this to the Execution Environment milestone Jan 22, 2024
@benesjan benesjan mentioned this issue Feb 6, 2024
Merged
benesjan added a commit that referenced this issue Feb 6, 2024
@benesjan
refactor: eth address tech debt cleanup (#4442)
153989f 
We used to serialize EthAddress as 32 bytes because we needed that for
compatibility with CBINDs. Given that we no longer use the C++ code this
is no longer necessary.

**Note**: decided to not completely nuke `EthAddress.toBuffer32()`
because that would require updating how we compute calldata/body hash
and it makes sense to do that once we'll do that in #3938 (linked it in
the issue).
This was referenced Feb 7, 2024
Closed
Closed
TomAFrench pushed a commit that referenced this issue Feb 7, 2024
@benesjan @TomAFrench
refactor: eth address tech debt cleanup (#4442)
fa5f66c 
We used to serialize EthAddress as 32 bytes because we needed that for
compatibility with CBINDs. Given that we no longer use the C++ code this
is no longer necessary.

**Note**: decided to not completely nuke `EthAddress.toBuffer32()`
because that would require updating how we compute calldata/body hash
and it makes sense to do that once we'll do that in #3938 (linked it in
the issue).
@LHerskind LHerskind changed the title Update the body hashing (circuits + l1contracts) + add TxEffect refactor: Update the TxsHash + add TxEffect Feb 12, 2024
@benesjan benesjan assigned benesjan and unassigned sklppy88 Feb 22, 2024
This was referenced Feb 26, 2024
Closed
Closed
michaelelliot pushed a commit to Swoir/noir_rs that referenced this issue Feb 28, 2024
@benesjan
refactor: eth address tech debt cleanup (AztecProtocol#4442)
520d316 
We used to serialize EthAddress as 32 bytes because we needed that for
compatibility with CBINDs. Given that we no longer use the C++ code this
is no longer necessary.

**Note**: decided to not completely nuke `EthAddress.toBuffer32()`
because that would require updating how we compute calldata/body hash
and it makes sense to do that once we'll do that in AztecProtocol#3938 (linked it in
the issue).
@LHerskind LHerskind mentioned this issue Feb 28, 2024
Merged
@benesjan benesjan mentioned this issue Mar 1, 2024
Merged
@benesjan
Copy link
Contributor Author

benesjan commented Mar 1, 2024

Addressed in #4726

@benesjan benesjan closed this as completed Mar 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@benesjan benesjan

Labels
None yet
Projects
A3
Archived in project
Milestone
Execution Environment pre offsite
Development

Successfully merging a pull request may close this issue.

refactor: updating encoding of TxEffects AztecProtocol/aztec-packages
3 participants
@benesjan @LHerskind @sklppy88