Skip to content
This repository was archived by the owner on Apr 18, 2025. It is now read-only.

Conversation

@DreamWuGit
Copy link

@DreamWuGit DreamWuGit commented May 31, 2024

Description

update TxL1FeeGadgetTestContainer to support Curie l1 fee unit tests

Issue Link

[link issue here]

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

@DreamWuGit DreamWuGit marked this pull request as ready for review June 3, 2024 01:25
@lispc
Copy link

lispc commented Jun 4, 2024

@DreamWuGit can merge develop

@DreamWuGit
Copy link
Author

will fix new test failure

@DreamWuGit DreamWuGit marked this pull request as draft June 4, 2024 07:55
@DreamWuGit DreamWuGit marked this pull request as ready for review June 5, 2024 01:29
@DreamWuGit
Copy link
Author

@lispc ready now.

.assign(region, 0, Value::known(F::from(is_curie)))?;
let rlp_signed_len = if is_curie == 1 { tx_signed_length } else { 0 };
self.tx_signed_length
.assign(region, 0, Value::known(F::from(rlp_signed_len)))?;
Copy link

Choose a reason for hiding this comment

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

i think it is ok to still assign tx_signed_length even when is_curie == 0? Not used anyway

Copy link
Author

Choose a reason for hiding this comment

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

yes

Copy link
Author

@DreamWuGit DreamWuGit Jun 5, 2024

Choose a reason for hiding this comment

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

in begin_tx , this tx_signed_length is looked up from tx table, so can not be any value eventually in real circuit even not is_curie.

@lispc lispc merged commit 8cd233b into develop Jun 5, 2024
@lispc lispc deleted the curie_unittest branch June 5, 2024 09:21
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants