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

Return CheckID from CheckCreate transaction #4898

Open
wants to merge 9 commits into
base: develop
Choose a base branch
from

Conversation

ckeshava
Copy link
Collaborator

High Level Overview of Change

This is an attempt to address the concerns raised in XRPLF/xrpl-py#356

Many thanks to @ximinez for brain-storming with me and giving me these ideas.

It is useful to return the CheckID after a successful CheckCreate transaction. At the moment, developers need to make multiple network calls (or) perform the necessary computation to calculate the CheckID. That is cumbersome.

I've built this branch off of #4864. The referenced work has necessary refactor of the submit command.

Context of Change

CheckCreate transaction returns the CheckID upon successful completion. CheckID is placed at the top level of the response, along with other fields like status, engine_result, etc.

I have also modified the unit tests to accomodate return values from the submit RPC command. Up until now, we were not able to access or validate the responses of the submit command within the Env framework.

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)
  • Refactor (non-breaking change that only restructures code)
  • Performance (increase or change in throughput and/or latency)
  • Tests (you added tests for code that already exists, or your new feature included in this PR)
  • Documentation update
  • Chore (no impact to binary, e.g. .gitignore, formatting, dropping support for older tooling)
  • Release

API Impact

  • Public API: New feature (new methods and/or new fields)
  • Public API: Breaking change (in general, breaking changes should only impact the next api_version)
  • libxrpl change (any change that may affect libxrpl or dependents of libxrpl)
  • Peer protocol change (must be backward compatible or bump the peer protocol version)

Performance Impact: This PR does not affect Performance of the rippled codebase

Test Plan

I've updated two of the existing unit tests to validate the output of CheckCreate transaction.

@ximinez @mDuo13 Please let me know if this looks okay

@ckeshava ckeshava changed the title New check Return CheckID from CheckCreate transaction Jan 24, 2024
@codecov-commenter
Copy link

Codecov Report

Attention: 13 lines in your changes are missing coverage. Please review.

Comparison is base (1676e9f) 61.50% compared to head (d4d0f62) 61.47%.

Files Patch % Lines
src/ripple/rpc/handlers/Submit.cpp 67.74% 7 Missing and 3 partials ⚠️
src/ripple/rpc/impl/TransactionSign.cpp 80.00% 0 Missing and 3 partials ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #4898      +/-   ##
===========================================
- Coverage    61.50%   61.47%   -0.04%     
===========================================
  Files          797      797              
  Lines        70123    70155      +32     
  Branches     36238    36261      +23     
===========================================
- Hits         43132    43129       -3     
- Misses       19755    19780      +25     
- Partials      7236     7246      +10     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

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

Successfully merging this pull request may close these issues.

None yet

2 participants