Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

t8n: Add tx hash calculation for tx receipt #590

Merged
merged 1 commit into from
Mar 22, 2023
Merged

Conversation

rodiazet
Copy link
Contributor

@rodiazet rodiazet commented Mar 21, 2023

  • Change is required by execution-spec-tests. The tool doesn't set hash field for transaction in txs.json. We have to calculate it.
  • Add better error printing to console.

@rodiazet rodiazet requested a review from chfast March 21, 2023 15:58
Copy link
Member

@axic axic left a comment

Choose a reason for hiding this comment

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

Looks good to me otherwise.

@axic axic changed the title Add tx hash calculation for tx receipt t8n: Add tx hash calculation for tx receipt Mar 22, 2023
@rodiazet rodiazet requested a review from chfast March 22, 2023 11:48
@rodiazet rodiazet requested a review from chfast March 22, 2023 14:33
@rodiazet rodiazet merged commit 61ce7c6 into master Mar 22, 2023
@rodiazet rodiazet deleted the t8n-improvement branch March 22, 2023 16:27
if (loaded_tx_hash_opt != computed_tx_hash)
throw std::logic_error("transaction hash mismatched: computed " +
hex0x(computed_tx_hash) + ", expected " +
hex0x(loaded_tx_hash_opt.value()));
Copy link
Member

@axic axic Mar 22, 2023

Choose a reason for hiding this comment

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

Could have used this here, no?

Suggested change
hex0x(loaded_tx_hash_opt.value()));
hex0x(*loaded_tx_hash_opt));

Copy link
Contributor Author

@rodiazet rodiazet Mar 22, 2023

Choose a reason for hiding this comment

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

Merged. I will add new PR if necessary

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

Successfully merging this pull request may close these issues.

3 participants