-
Notifications
You must be signed in to change notification settings - Fork 947
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
split transfer tx #3297
split transfer tx #3297
Conversation
a49637e
to
e27a570
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3297 +/- ##
==========================================
- Coverage 53.93% 53.57% -0.36%
==========================================
Files 314 316 +2
Lines 105739 106457 +718
==========================================
+ Hits 57027 57031 +4
- Misses 48712 49426 +714 ☔ View full report in Codecov by Sentry. |
e27a570
to
cd09a6b
Compare
cf4e9a8
to
aa45a78
Compare
crates/apps_lib/src/client/tx.rs
Outdated
args: args::TxShieldingTransfer, | ||
) -> Result<(), error::Error> { | ||
// Repeat once if the tx fails on a crossover of an epoch | ||
for _ in 0..2 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe this loop is only needed for shielding transactions but we don't need it when doing shielded or unshielding. Maybe @murisi can confirm this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe this loop is only needed for shielding transactions but we don't need it when doing shielded or unshielding. Maybe @murisi can confirm this
Yes, agreed. You cannot submit a shielding transaction in later epochs because users would exploit this by pretending the transactions were submitted much earlier and claim a large reward. But late submissions are permitted for shielded and unshielding because there's nothing to be gained by the user by doing so.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thx! Fixed in 3e61a3a
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added one more commit to remove this from shielded transfer too - https://github.com/anoma/namada/pull/3297/files/838cec13b9d1d4b71f864b526be7addbab1d80e5..7ca37179029644c06a4f8f003e23fc3cb98c6503
The merge-base changed after approval.
* origin/tomas/split-transfer-tx: update hermes ShieldingTransfer for IBC messages fixup! sdk: update for split-up transfer tx fixup! tests: update for split-up transfer tx changelog: add #3297 sdk/tx: fix the display of insufficient funds for non-native token wasm_for_tests: update Cargo.lock wasm/tx_ibc: update the shielded transfer benches: update for split-up transfer tx encoding_spec: update for split-up transfer tx tests: update for split-up transfer tx node/bench_utils: update for split-up transfer tx apps: update for split-up transfer tx apps_lib: update for split-up transfer tx light_sdk: update for split-up transfer tx namada/protocol: rm dead code namada: update ibc path imports sdk: update for split-up transfer tx wasm: add shielded, shielding and unshielding transfer wasm/tx_transparent_transfer: remove the shielded section wasm: rename `tx_transfer` to `tx_transparent_transfer` move most of core IBC types into IBC crate replace the single `Transfer` struct with 4 distinct ones
* up/tomas/split-transfer-tx: retry tx submission on shielding fixup! apps_lib: update for split-up transfer tx fixup! apps_lib: update for split-up transfer tx update hermes ShieldingTransfer for IBC messages fixup! sdk: update for split-up transfer tx fixup! tests: update for split-up transfer tx changelog: add #3297 sdk/tx: fix the display of insufficient funds for non-native token wasm_for_tests: update Cargo.lock wasm/tx_ibc: update the shielded transfer benches: update for split-up transfer tx encoding_spec: update for split-up transfer tx tests: update for split-up transfer tx node/bench_utils: update for split-up transfer tx apps: update for split-up transfer tx apps_lib: update for split-up transfer tx light_sdk: update for split-up transfer tx namada/protocol: rm dead code namada: update ibc path imports sdk: update for split-up transfer tx wasm: add shielded, shielding and unshielding transfer wasm/tx_transparent_transfer: remove the shielded section wasm: rename `tx_transfer` to `tx_transparent_transfer` move most of core IBC types into IBC crate replace the single `Transfer` struct with 4 distinct ones
21c9d36
to
ff29e1c
Compare
auto-squashed fixups |
Describe your changes
closes #1677, closes #3345
The transfer CLI command is split into:
transfer
(shielded)transparent-transfer
shield
(from transparent to shielded)unshield
(from shielded to transparent)With a dedicated wasm tx for each case, respectively:
tx_shielded_transfer
tx_transparent_transfer
tx_shielding_transfer
tx_unshielding_transfer
Indicate on which release or other PRs this topic is based on
#3232 (based on 0.37)
Checklist before merging to
draft