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

Remove wrapper fee unshielding #3217

Merged
merged 83 commits into from
May 21, 2024
Merged

Remove wrapper fee unshielding #3217

merged 83 commits into from
May 21, 2024

Conversation

grarco
Copy link
Contributor

@grarco grarco commented May 9, 2024

Describe your changes

Partially addresses #2597.

Removes the optional fee unshielding Transaction object from WrapperTx. Removes/refactors associated functions, types and tests. Removes fee unshielding support from the client and the sdk.

NOTE: with this PR only transactions that pay fees from the signer balance are accepted. If you need to test masp transactions you'll need to have enough funds in a transparent balance.

Indicate on which release or other PRs this topic is based on

#3103 (diffs for review: https://github.com/anoma/namada/pull/3217/files/7f4b6c1e6656ec44194562ad1e167b09d24170f4..cf52c605e33279f4f464b210a8e215a7138be809)

Checklist before merging to draft

  • I have added a changelog
  • Git history is in acceptable state

batconjurer
batconjurer previously approved these changes May 16, 2024
Copy link
Member

@batconjurer batconjurer left a comment

Choose a reason for hiding this comment

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

Apart from a complaint I have about the pr this is based on top of, this looks fine to me.

@@ -327,7 +327,6 @@ impl CliApi {
chain_ctx.shielded,
&client,
&io,
args.batch_size,
Copy link
Member

Choose a reason for hiding this comment

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

again, this shouldn't be removed.

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 should be fixed now in the base PR

brentstone added a commit that referenced this pull request May 16, 2024
* grarco/masp-fees:
  Changelog #3217
  Assigns issues to TODOs
  Removes `wrapper_changed_keys` from `TxResult`
  Removes fee unshielding e2e test
  Removes fee unshielding integration tests
  Recomputes tx test fixture
  Recomputes wasm for tests modules
  Updates TODO comments
  Reworks sdk fee validation
  Renames `wrapper_fee_check`
  Removes error/functions related to fee unshielding
  Removes `descriptions_limit` protocol param
  Removes redundandt wrapper types
  Removes fee unshielding cli and tx args
  Removes `unshield_section_hash` from `WrapperTx`. Updates the relative functions. Removes event for masp wrapper tx
tzemanovic added a commit that referenced this pull request May 20, 2024
* origin/grarco/masp-fees:
  Changelog #3217
  Assigns issues to TODOs
  Removes `wrapper_changed_keys` from `TxResult`
  Removes fee unshielding e2e test
  Removes fee unshielding integration tests
  Recomputes tx test fixture
  Recomputes wasm for tests modules
  Updates TODO comments
  Reworks sdk fee validation
  Renames `wrapper_fee_check`
  Removes error/functions related to fee unshielding
  Removes `descriptions_limit` protocol param
  Removes redundandt wrapper types
  Removes fee unshielding cli and tx args
  Removes `unshield_section_hash` from `WrapperTx`. Updates the relative functions. Removes event for masp wrapper tx
brentstone added a commit that referenced this pull request May 21, 2024
* origin/grarco/masp-fees:
  Changelog #3217
  Assigns issues to TODOs
  Removes `wrapper_changed_keys` from `TxResult`
  Removes fee unshielding e2e test
  Removes fee unshielding integration tests
  Recomputes tx test fixture
  Recomputes wasm for tests modules
  Updates TODO comments
  Reworks sdk fee validation
  Renames `wrapper_fee_check`
  Removes error/functions related to fee unshielding
  Removes `descriptions_limit` protocol param
  Removes redundandt wrapper types
  Removes fee unshielding cli and tx args
  Removes `unshield_section_hash` from `WrapperTx`. Updates the relative functions. Removes event for masp wrapper tx
tzemanovic added a commit that referenced this pull request May 21, 2024
* tomas/split-up-apps:
  changelog: add #3259
  test/apps_lib: fix the top-level dir check
  fix paths for split up namada_apps_lib
  git: ignore the new generated version.rs path
  symlink proto from `apps_lib`
  `mv crates/apps/build.rs crates/apps_lib/`
  `mv crates/apps_lib/src/lib/mod.rs crates/apps_lib/src/lib.rs`
  `mv crates/apps/src/lib crates/apps_lib/src`
  apps_lib: add a new crate for apps lib crate
  fixup! Merge branch 'grarco/masp-fees' (#3217)
  fixup! Merge branch 'grarco/tx-batch' (#3103)
  fixup! Merge branch 'grarco/tx-batch' (#3103)
  fixup! Merge branch 'bat/fix/issue-1796' (#3226)
  fixup! Merge branch 'tiago/max-proposal-bytes-validation' (#3220)
  fixup! Merge branch 'tomas/more-checked-arith' (#3214)
  empty commit
  changelog: add #3216
  deliberatly empty
  Changelog #2817
  added clone to transaction structs
@tzemanovic tzemanovic merged commit 5869a48 into main May 21, 2024
12 of 19 checks passed
@tzemanovic tzemanovic deleted the grarco/masp-fees branch May 21, 2024 13:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants