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

Assess and memoize (if appropriate) SignedTransaction::sha256_of_proto_encoding #1352

Open
Fraser999 opened this issue Aug 7, 2024 · 2 comments
Assignees
Labels
ENG-667 ignore-stale Override for issues or PRs which should not be removed if stale. performance stale tech-debt

Comments

@Fraser999
Copy link
Contributor

Fraser999 commented Aug 7, 2024

We should run an ad hoc test under semi-realistic conditions which counts how many times SignedTransaction::sha256_of_proto_encoding is a repeated call on a SignedTransaction instance. In other words, if we only ever call this once per instance, the total will be zero. If we call it exactly thrice per instance, the total will be twice the number of instances.

Once we have this information, we can decide whether to implement memoization of the tx hash as a field in the tx, or leave as is.

After brief discussion with @SuperFluffy, it might be better to just make this hash a normal member field rather than a lazily-initialized OnceCell for example, since the hash is probably almost always calculated for every signed transaction. Confer before implementing any changes to agree on an approach.

┆Issue Number: ENG-697

@joroshiba
Copy link
Member

This issue is stale because it has been open 45 days with no activity. Remove stale label or this issue
be closed in 7 days.

@Fraser999 Fraser999 added ignore-stale Override for issues or PRs which should not be removed if stale. and removed ignore-stale Override for issues or PRs which should not be removed if stale. stale labels Dec 16, 2024
@SuperFluffy SuperFluffy added the ignore-stale Override for issues or PRs which should not be removed if stale. label Jan 30, 2025
@SuperFluffy
Copy link
Member

We should just hash the signed transaction upon construction and not worry about memoization - I can't think of a scenario where we would construct a signed transaction without immediately logging its hash in some form.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ENG-667 ignore-stale Override for issues or PRs which should not be removed if stale. performance stale tech-debt
Projects
None yet
Development

No branches or pull requests

4 participants