Skip to content

Conversation

@grarco
Copy link
Collaborator

@grarco grarco commented Oct 7, 2024

Describe your changes

Closes #3875.

Updates the client and the SDK (in a breaking manner) to:

  • Selectively dump a raw or a wrapper tx
  • Produce an offline signature for a wrapper transaction (gas payment)
  • Allow loading a wrapper signature when building a custom tx

Some misc updates to allow the ones above:

  • Reworked requirements and conflicts of some CLI args
  • Changed the owner arg to be optional for custom tx and offline signing
  • build_custom optionally returns SigningTxData (it returns None if all the signatures have been provided from file)
  • Updated tests

I've also opened #3901 as a possible followup to this PR.

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
  • If this PR requires changes to the docs or specs, a corresponding PR is opened in the namada-docs repo
    • Relevant PR if applies:
  • If this PR affects services such as namada-indexer or namada-masp-indexer, a corresponding PR is opened in that repo
    • Relevant PR if applies:

@grarco grarco added breaking:client Namada client breaking change breaking:SDK SDK breaking change labels Oct 7, 2024
@grarco grarco force-pushed the grarco/offline-wrapper-signature branch from 9b4538b to 3840987 Compare October 8, 2024 09:00
grarco added a commit that referenced this pull request Oct 8, 2024
@grarco grarco marked this pull request as ready for review October 8, 2024 15:06
@codecov
Copy link

codecov bot commented Oct 8, 2024

Codecov Report

Attention: Patch coverage is 84.94624% with 14 lines in your changes missing coverage. Please review.

Project coverage is 73.99%. Comparing base (c465b7d) to head (f80e02e).
Report is 20 commits behind head on main.

Files with missing lines Patch % Lines
crates/sdk/src/tx.rs 90.19% 5 Missing ⚠️
crates/sdk/src/lib.rs 40.00% 3 Missing ⚠️
crates/sdk/src/signing.rs 90.90% 3 Missing ⚠️
crates/apps_lib/src/config/genesis/transactions.rs 0.00% 2 Missing ⚠️
crates/sdk/src/args.rs 50.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3900      +/-   ##
==========================================
- Coverage   74.02%   73.99%   -0.04%     
==========================================
  Files         341      341              
  Lines      106560   106608      +48     
==========================================
+ Hits        78879    78880       +1     
- Misses      27681    27728      +47     

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

@grarco grarco force-pushed the grarco/offline-wrapper-signature branch from 14147c8 to 39f2c11 Compare October 22, 2024 14:52
@Fraccaman Fraccaman added the merge Ready to merge - mergifyio bot will add the PR to merge queue when all checks pass label Oct 23, 2024
@mergify
Copy link
Contributor

mergify bot commented Oct 23, 2024

Merge Protections

Your pull request matches the following merge protections and will not be merged until they are valid.

🛡 GitHub branch protections and repository rulesets requirements

  • #approved-reviews-by >= 1 [🛡 GitHub branch protection]
  • #changes-requested-reviews-by = 0 [🛡 GitHub branch protection]
  • any of: [🛡 GitHub branch protection]
    • check-success = test-e2e-with-device-automation
    • check-neutral = test-e2e-with-device-automation
    • check-skipped = test-e2e-with-device-automation
  • any of: [🛡 GitHub branch protection]
    • check-success = check-benchmarks
    • check-neutral = check-benchmarks
    • check-skipped = check-benchmarks
  • any of: [🛡 GitHub branch protection]
    • check-success = check-packages
    • check-neutral = check-packages
    • check-skipped = check-packages
  • any of: [🛡 GitHub branch protection]
    • check-success = build-binaries
    • check-neutral = build-binaries
    • check-skipped = build-binaries
  • any of: [🛡 GitHub branch protection]
    • check-success = build-wasm (wasm-for-tests, build-wasm-tests-scripts, wasm_for_tests/*.wasm)
    • check-neutral = build-wasm (wasm-for-tests, build-wasm-tests-scripts, wasm_for_tests/*.wasm)
    • check-skipped = build-wasm (wasm-for-tests, build-wasm-tests-scripts, wasm_for_tests/*.wasm)
  • any of: [🛡 GitHub branch protection]
    • check-success = lints
    • check-neutral = lints
    • check-skipped = lints
  • any of: [🛡 GitHub branch protection]
    • check-success = rust-docs
    • check-neutral = rust-docs
    • check-skipped = rust-docs
  • any of: [🛡 GitHub branch protection]
    • check-success = test-wasm
    • check-neutral = test-wasm
    • check-skipped = test-wasm
  • any of: [🛡 GitHub branch protection]
    • check-success = test-e2e (0)
    • check-neutral = test-e2e (0)
    • check-skipped = test-e2e (0)
  • any of: [🛡 GitHub branch protection]
    • check-success = test-e2e (1)
    • check-neutral = test-e2e (1)
    • check-skipped = test-e2e (1)
  • any of: [🛡 GitHub branch protection]
    • check-success = test-e2e (2)
    • check-neutral = test-e2e (2)
    • check-skipped = test-e2e (2)
  • any of: [🛡 GitHub branch protection]
    • check-success = test-e2e (3)
    • check-neutral = test-e2e (3)
    • check-skipped = test-e2e (3)
  • any of: [🛡 GitHub branch protection]
    • check-success = test-e2e (4)
    • check-neutral = test-e2e (4)
    • check-skipped = test-e2e (4)
  • any of: [🛡 GitHub branch protection]
    • check-success = test-integration
    • check-neutral = test-integration
    • check-skipped = test-integration
  • any of: [🛡 GitHub branch protection]
    • check-success = test-unit
    • check-neutral = test-unit
    • check-skipped = test-unit

@mergify mergify bot merged commit f722c1c into main Oct 23, 2024
23 of 24 checks passed
@mergify mergify bot deleted the grarco/offline-wrapper-signature branch October 23, 2024 13:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking:client Namada client breaking change breaking:SDK SDK breaking change 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.

Namadac tx broadcasting bugs

3 participants