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

improve offline tx handling #3715

Merged
merged 9 commits into from
Sep 6, 2024
Merged

Conversation

Fraccaman
Copy link
Member

@Fraccaman Fraccaman commented Aug 28, 2024

Describe your changes

Closes #3417.

Checklist before merging

  • If this PR has some consensus breaking changes, I added the corresponding breaking:: labels
    • This will require 2 reviewers to approve the changes

@Fraccaman Fraccaman changed the title Fraccaman/improve offline tx handling improve offline tx handling Aug 28, 2024
Copy link

codecov bot commented Aug 28, 2024

Codecov Report

Attention: Patch coverage is 86.48649% with 20 lines in your changes missing coverage. Please review.

Project coverage is 72.68%. Comparing base (521511f) to head (34c6ac4).
Report is 13 commits behind head on main.

Files with missing lines Patch % Lines
crates/core/src/key/common.rs 79.48% 8 Missing ⚠️
crates/core/src/hash.rs 84.84% 5 Missing ⚠️
crates/tx/src/lib.rs 91.37% 5 Missing ⚠️
crates/sdk/src/tx.rs 80.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3715      +/-   ##
==========================================
+ Coverage   72.49%   72.68%   +0.19%     
==========================================
  Files         338      338              
  Lines      104190   104309     +119     
==========================================
+ Hits        75535    75822     +287     
+ Misses      28655    28487     -168     

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

@Fraccaman Fraccaman requested review from sug0 and tzemanovic and removed request for sug0 August 28, 2024 11:14
@Fraccaman Fraccaman marked this pull request as ready for review August 28, 2024 11:14
@Fraccaman Fraccaman requested a review from sug0 August 28, 2024 11:14
Copy link
Contributor

@sug0 sug0 left a comment

Choose a reason for hiding this comment

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

just a couple of suggestions

crates/apps_lib/src/cli.rs Outdated Show resolved Hide resolved
crates/core/src/hash.rs Outdated Show resolved Hide resolved
crates/tx/src/lib.rs Outdated Show resolved Hide resolved
@Fraccaman Fraccaman force-pushed the fraccaman/improve-offline-tx-handling branch 2 times, most recently from c2d4a48 to edfb095 Compare August 28, 2024 16:09
@tzemanovic tzemanovic force-pushed the fraccaman/improve-offline-tx-handling branch from 9cd2bc0 to affb5b2 Compare September 2, 2024 17:24
@tzemanovic tzemanovic added the merge Ready to merge - mergifyio bot will add the PR to merge queue when all checks pass label Sep 2, 2024
@sug0
Copy link
Contributor

sug0 commented Sep 2, 2024

    integration::ledger_tests::pgf_governance_proposal

failed

@tzemanovic
Copy link
Member

yeah, seeing this in #3718 and #3735 after rebase too

@Fraccaman Fraccaman force-pushed the fraccaman/improve-offline-tx-handling branch from affb5b2 to 0a33340 Compare September 4, 2024 09:56
@dan-u410
Copy link

dan-u410 commented Sep 5, 2024

there's another (broken) offline signing cmd, namada client sign-tx might be nice to deprecate that in favor of sign-offline being developed here

@brentstone brentstone force-pushed the fraccaman/improve-offline-tx-handling branch from 0a33340 to 34c6ac4 Compare September 6, 2024 14:31
mergify bot added a commit that referenced this pull request Sep 6, 2024
mergify bot added a commit that referenced this pull request Sep 6, 2024
mergify bot added a commit that referenced this pull request Sep 6, 2024
mergify bot added a commit that referenced this pull request Sep 6, 2024
@mergify mergify bot merged commit c3d1a8f into main Sep 6, 2024
23 checks passed
@mergify mergify bot deleted the fraccaman/improve-offline-tx-handling branch September 6, 2024 15:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merge Ready to merge - mergifyio bot will add the PR to merge queue when all checks pass
Projects
None yet
Development

Successfully merging this pull request may close these issues.

dump-tx invalid serialization
4 participants