Add other confidential tx methods to seid #1933
Conversation
| @@ -358,6 +358,10 @@ func (m msgServer) Transfer(goCtx context.Context, req *types.MsgTransfer) (*typ | |||
| return nil, sdkerrors.Wrap(sdkerrors.ErrInvalidRequest, "invalid msg") | |||
| } | |||
|
|
|||
| if instruction.FromAddress == instruction.ToAddress { | |||
There was a problem hiding this comment.
I guess I agree, but In bank module though, I think it's a valid case
There was a problem hiding this comment.
Disabled this because the way we write this is
senderState = getAccount(sender)
recipientState = getAccount(recipient)
senderState.Balance += Amount
recipientState.Balance -= Amount
keeper.Store(senderState, sender)
keeper.Store(recipientState, recipient).
If sender and recipient are same address, the updated 'recipientState' overrides the updated 'senderState' for the key 'sender' and funds are not deducted. If we want to enable this we need to write a special case.
| cmd := &cobra.Command{ | ||
| Use: "transfer [denom] [to_address] [amount] [flags]", | ||
| Short: "Make a confidential transfer to another address", | ||
| Long: `Transfer command create a confidential transfer of the specified amount of the specified denomination to the specified address. |
There was a problem hiding this comment.
nit: We could also add some example commands in long description. I think i rushed my piece in without it though
|
|
||
| fromAddress := clientCtx.GetFromAddress().String() | ||
| denom := args[0] | ||
| toAddress := args[1] |
There was a problem hiding this comment.
we may want to validate inputs. I think I skipped those checks in commands I implemented, and we may need to revisit here and other places
There was a problem hiding this comment.
Updated the commands with denom and address validation
|
Code LGTM in general. Let's fix lint errors and add unit tests where possible. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## feature/ct_types #1933 +/- ##
====================================================
- Coverage 61.57% 60.13% -1.45%
====================================================
Files 263 281 +18
Lines 23306 25369 +2063
====================================================
+ Hits 14351 15255 +904
- Misses 7948 8982 +1034
- Partials 1007 1132 +125
|
| @@ -17,3 +17,11 @@ func SplitTransferBalance(amount uint64) (uint16, uint32, error) { | |||
|
|
|||
| return bottom16, next32, nil | |||
| } | |||
|
|
|||
| // Combines the bottom 32 bits and the next 48 bits into a single 64-bit number. | |||
There was a problem hiding this comment.
nit: godoc usually stars with method name e.g
// CombineTransferAmount Combines the bottom 32 bits and the next 48 bits into a single 64-bit number.
| @@ -17,3 +17,11 @@ func SplitTransferBalance(amount uint64) (uint16, uint32, error) { | |||
|
|
|||
| return bottom16, next32, nil | |||
| } | |||
|
|
|||
| // Combines the bottom 32 bits and the next 48 bits into a single 64-bit number. | |||
| func CombineTransferAmount(bottom16 uint16, hi uint32) (uint64, error) { | |||
There was a problem hiding this comment.
Probably deserves it's own unit test
* add other tx commands * small fixes * add param validation * fix apply pending balance method * check denom for initialize * add util tests * bug * fix initialize tests
Describe your changes and provide context
Adds the remaining 4 commands
Testing performed to validate your change
Manual testing with the seid cli