Skip to content

Conversation

@grarco
Copy link
Collaborator

@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
Collaborator

@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.

chain_ctx.shielded,
&client,
&io,
args.batch_size,
Copy link
Collaborator

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
Collaborator 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

@grarco grarco force-pushed the grarco/masp-fees branch from cf52c60 to c2e98ab Compare May 16, 2024 09:50
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
@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

Labels

breaking:consensus Consensus breaking change that requires a hard-fork breaking: tx Transaction format breaking change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants