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

Ensure comments actually update in a transaction log #656

Merged
merged 5 commits into from
Jan 23, 2023

Conversation

pouneh
Copy link
Contributor

@pouneh pouneh commented Jan 10, 2023

Motivation

When a field is provided in an API, we expect that field to be leveraged. Specifically, in this case, the submit_transaction API wasn't using the provided comment to update the comment field in an existing transaction log.

In this PR

We update the comment both when signing the transaction, and when submitting it.

Test Plan

To run the rust test suite (including the test added in this PR), copy the consensus_enclave.css file to your current working directory, and then run tools/test.sh

Manually, you can exercise the scenario by doing the following

  1. Create a transaction with the build_transaction API
  2. Grab the transaction_log_id from the response
  3. Check the transaction log with the get_transaction_log API using the transaction_log_id from Step 2
  4. Observe that the comment field in get_transaction_log's response is an empty string
  5. Submit the transaction using the submit_transaction API, and specify the optional parameter comment to be a string of your choice
  6. Check the transaction log with the get_transaction_log API using the transaction_log_id from Step 2 and observe your new comment!

@pouneh pouneh changed the title Bugfix/transactionlog comments/183870444 Ensure comments actually update in a transaction log Jan 10, 2023
@pouneh pouneh force-pushed the bugfix/transactionlog_comments/183870444 branch from 1caafcf to 0fe7bff Compare January 10, 2023 18:36
@briancorbin briancorbin added the bug Something isn't working label Jan 10, 2023
@pouneh pouneh merged commit 0f2edd1 into develop Jan 23, 2023
@pouneh pouneh deleted the bugfix/transactionlog_comments/183870444 branch January 23, 2023 21:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants