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

Implement txn endorsing for IndyVdrLedger #852

Merged
merged 7 commits into from
May 19, 2023
Merged

Conversation

mirgee
Copy link
Contributor

@mirgee mirgee commented May 18, 2023

Implements endorse_transaction, set_endorser and get_txn_author_agreement for IndyVdrLedger and enables test_endorse_transaction test.

Signed-off-by: Miroslav Kovar <miroslav.kovar@absa.africa>
@codecov-commenter
Copy link

codecov-commenter commented May 18, 2023

Codecov Report

Merging #852 (200613d) into main (e8233f4) will decrease coverage by 0.02%.
The diff coverage is 0.00%.

@@           Coverage Diff            @@
##            main    #852      +/-   ##
========================================
- Coverage   8.70%   8.69%   -0.02%     
========================================
  Files        408     408              
  Lines      32868   32911      +43     
  Branches    7207    7215       +8     
========================================
  Hits        2861    2861              
- Misses     29196   29239      +43     
  Partials     811     811              
Flag Coverage Δ
unittests-aries-vcx 8.69% <0.00%> (-0.02%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
aries_vcx/tests/test_pool.rs 0.56% <0.00%> (-0.09%) ⬇️
aries_vcx_core/src/common/ledger/transactions.rs 0.00% <0.00%> (ø)
aries_vcx_core/src/indy/ledger/transactions.rs 6.90% <0.00%> (+0.28%) ⬆️

... and 1 file with indirect coverage changes

@mirgee mirgee marked this pull request as ready for review May 18, 2023 08:20
@mirgee mirgee force-pushed the feature/endorse-txn branch 2 times, most recently from e15f5a4 to 56640e6 Compare May 18, 2023 08:53
Copy link
Contributor

@Patrik-Stas Patrik-Stas left a comment

Choose a reason for hiding this comment

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

left review

aries_vcx/tests/test_pool.rs Outdated Show resolved Hide resolved
aries_vcx_core/src/common/ledger/transactions.rs Outdated Show resolved Hide resolved
aries_vcx/tests/test_pool.rs Show resolved Hide resolved
aries_vcx_core/src/ledger/indy_vdr_ledger.rs Outdated Show resolved Hide resolved
Signed-off-by: Miroslav Kovar <miroslav.kovar@absa.africa>
Signed-off-by: Miroslav Kovar <miroslav.kovar@absa.africa>
Signed-off-by: Miroslav Kovar <miroslav.kovar@absa.africa>
Signed-off-by: Miroslav Kovar <miroslav.kovar@absa.africa>
Signed-off-by: Miroslav Kovar <miroslav.kovar@absa.africa>
Ok(())
let mut request = self._build_schema_request(submitter_did, schema_json)?;
request = _append_txn_author_agreement_to_request(request).await?;
if let Some(endorser_did) = endorser_did {
Copy link
Contributor

Choose a reason for hiding this comment

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

this endorsing sequence is missing for other publish_* methods, but I see that we don't even pass endorses_did Option in those. Okay, I guess that could be another github issue (perhaps even good-first-issue, should be fairly simple)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is just replicating the current vdrtools implementation. They should behave identically and fixes like this should be done on both until we remove the vdrtools one.

Patrik-Stas
Patrik-Stas previously approved these changes May 19, 2023
Copy link
Contributor

@Patrik-Stas Patrik-Stas left a comment

Choose a reason for hiding this comment

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

lgtm

Base automatically changed from feature/ledger-indy to main May 19, 2023 11:58
Signed-off-by: Miroslav Kovar <miroslav.kovar@absa.africa>

# Conflicts:
#	aries_vcx_core/src/ledger/indy_vdr_ledger.rs
@Patrik-Stas Patrik-Stas merged commit 77310eb into main May 19, 2023
41 checks passed
@Patrik-Stas Patrik-Stas deleted the feature/endorse-txn branch May 19, 2023 14:37
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

3 participants