Murisi/unconstrained transfers #3459
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3459 +/- ##
==========================================
- Coverage 53.92% 53.86% -0.07%
==========================================
Files 317 317
Lines 107575 107746 +171
==========================================
+ Hits 58011 58032 +21
- Misses 49564 49714 +150 ☔ View full report in Codecov by Sentry. |
murisi
force-pushed
the
murisi/unconstrained-transfers
branch
from
08025d9 to
7d2bdf7
Compare
cwgoes
reviewed
Jun 28, 2024
Collaborator
cwgoes
left a comment
cwgoes
left a comment
There was a problem hiding this comment.
Conceptual ACK, but this should receive careful review for correct arithmetic.
murisi
force-pushed
the
murisi/unconstrained-transfers
branch
from
7d2bdf7 to
424d0f6
Compare
2 tasks
grarco
reviewed
Jul 1, 2024
grarco
reviewed
Jul 1, 2024
grarco
reviewed
Jul 1, 2024
tzemanovic
reviewed
Jul 1, 2024
murisi
force-pushed
the
murisi/unconstrained-transfers
branch
from
424d0f6 to
67130f8
Compare
murisi
force-pushed
the
murisi/unconstrained-transfers
branch
2 times, most recently
from
43541f3 to
987253d
Compare
murisi
force-pushed
the
murisi/unconstrained-transfers
branch
from
987253d to
317b3ad
Compare
murisi
added a commit
that referenced
this pull request
Jul 1, 2024
Closed
tzemanovic
reviewed
Jul 2, 2024
tzemanovic
previously approved these changes
Jul 2, 2024
2 tasks
murisi
force-pushed
the
murisi/unconstrained-transfers
branch
from
e4da8c0 to
98c88ec
Compare
murisi
added a commit
that referenced
this pull request
Jul 2, 2024
sug0
reviewed
Jul 3, 2024
Comment on lines
+736
to
+753
| impl serde::Serialize for UserAccount { | ||
| fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error> | ||
| where | ||
| S: serde::Serializer, | ||
| { | ||
| serializer.collect_str(&self.to_string()) | ||
| } | ||
| } | ||
|
|
||
| impl<'de> Deserialize<'de> for UserAccount { | ||
| fn deserialize<D>(deserializer: D) -> Result<Self, D::Error> | ||
| where | ||
| D: Deserializer<'de>, | ||
| { | ||
| let s = <String as Deserialize>::deserialize(deserializer)?; | ||
| FromStr::from_str(&s).map_err(serde::de::Error::custom) | ||
| } | ||
| } |
Collaborator
There was a problem hiding this comment.
replace with namada::core::string_encoding::StringEncoded, which uses Display and FromStr to encode some value with serde
Collaborator
There was a problem hiding this comment.
StringEncoded<UserAccount> that is
Collaborator
There was a problem hiding this comment.
@murisi can we try to address any further items related to this PR in a follow-up PR? This has been merged to draft now.
Collaborator
Author
There was a problem hiding this comment.
Sure, understood. Thanks.
brentstone
added a commit
that referenced
this pull request
Jul 4, 2024
* origin/murisi/unconstrained-transfers: Generalized the Transfer event to support reporting multiple account changes. Unconstrain transfers and combine transfer amounts. Renamed some data structures. Added a changelog entry. Remove now dead code. Subsumed unshielding transfer into generalized transfer. Subsumed shielding transfer into generalized transfer. Subsumed shielded transfer into generalized transfer. Renamed TransparentTransfer to Transfer. Generalized the TransparentTransfer to support a shielded action.
brentstone
added a commit
that referenced
this pull request
Jul 4, 2024
brentstone
added a commit
that referenced
this pull request
Jul 4, 2024
5 tasks
2 tasks
3 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Describe your changes
An attempt to defragment the inputs and outputs of a transfer to ease the printing of transfers on hardware wallets. More specifically, the following changes have been made:
multi_transferfunction in the transaction prelude that can apply any number of balance changes simultaneously instead of just 2 like the existingtransferfunctionIndicate on which release or other PRs this topic is based on
#3446
Checklist before merging to
draft