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

Introduce new TransactionWithSignature interface #2175

Open
kirugan opened this issue Sep 24, 2024 · 1 comment
Open

Introduce new TransactionWithSignature interface #2175

kirugan opened this issue Sep 24, 2024 · 1 comment
Labels

Comments

@kirugan
Copy link
Contributor

kirugan commented Sep 24, 2024

Right now all transactions implement Signature() method, but we don't need it for L1Handler and Deploy transactions (right now they return empty slice).
We already faced some issues with it, here is an example:

if txSignature := transaction.Signature(); len(txSignature) > 0 {
   digest.Update(txSignature...)
} else {
   digest.Update(&felt.Zero)
}

it was rewritten as:

switch transaction.(type) {
  case *DeployTransaction, *L1HandlerTransaction:
     digest.Update(&felt.Zero)
  default:
    digest.Update(transaction.Signature()...)
}

but code above may lead to more questions: why we're specifying exactly these transactions? is it specific to this function only or applicable to entire codebase?

It can be solved by introducing new interface type TransactionWithSignature (name only for example, I'm pretty sure there is a better naming):

type TransactionWithSignature interface {
   Signature() []*felt.Felt
}

which can be used in example above as:

switch transaction.(type) {
case TransactionWithSignature:
     digest.Update(transaction.Signature()...)
default:
     digest.Update(&felt.Zero)
}
@pri-3x
Copy link

pri-3x commented Sep 30, 2024

@kirugan PR for this issue is opened: #2189
The workflows are awaiting approval to run. Could you please review and approve them?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants