Incorrect `Event::UnlockRedeemed` for`redeem`

[hats-bug-reporter[bot]]

Github username: @aktech297 Twitter username: kaka Submission hash (on-chain): 0xeaffe80c4597b40caa09aafbc76ba6fdddc7dd44a7a9250baf86821d7346f64d Severity: low

Description: Description\ The emitted parameters are incorrect. they are not based on the event struct definition.

Impact\ Incorrect event information notified to user. If thess are directly used in the front end application logic, it would bring unexpected issues.

Attachments

1. Proof of Concept (PoC) File

At the end of the redeem function, the processed elements such as staker, azero, unlock_id and the batch_id

https://github.com/hats-finance/Kintsu-0x7d70f9442af3a9a0a734fa6a1b4857f25518e9d2/blob/c9bdc853b18c305de832307b91a9bca0f281f71e/src/vault/lib.rs#L458-L466

            Self::emit_event(
                Self::env(),
                Event::UnlockRedeemed(UnlockRedeemed {
                    staker: user,
                    azero,
                    unlock_id,
                    batch_id,
                }),
            );

But the unlock_id and batch_id interchanged.

when we look at the UnlockRedeemedstruct definition.

lib.rs#L71-L78

    #[ink(event)]
    pub struct UnlockRedeemed {
        #[ink(topic)]
        staker: AccountId,
        azero: Balance,
        batch_id: u64,
        unlock_id: u64,
    }

in rust, the order of struct parameters can be interchanged only with the elements names.

2. Revised Code File (Optional)

we would suggest interchane the event emit values.

            Self::emit_event(
                Self::env(),
                Event::UnlockRedeemed(UnlockRedeemed {
                    staker: user,
                    azero,
                    batch_id,
                    unlock_id,
                }),
            );

[bgibers]

In Rust, the order of the fields in a struct does not matter when you are accessing them or initializing them, as long as you use the field names

[0xmahdirostami]

In Rust, the order of the fields in a struct does not matter when you are accessing them or initializing them, as long as you use the field names

SER the event is in following way:

    #[ink(event)]
    pub struct UnlockRedeemed {
        #[ink(topic)]
        staker: AccountId,
        azero: Balance,
        batch_id: u64,
        unlock_id: u64,
    }

and you are emitting it in the following way: (if you are not using the name, the order must be correct)

            Self::emit_event(
                Self::env(),
                Event::UnlockRedeemed(UnlockRedeemed {
                    staker: user,
                    azero,
                    unlock_id,
                    batch_id,
                }),
            );

for example: unlock_id is 2, batch_id is 3

            Self::emit_event(
                Self::env(),
                Event::UnlockRedeemed(UnlockRedeemed {
                    staker: user,
                    azero,
                    2,
                    3,
                }),
            );

please review it again, i think the issue is valid, and you should emit it in the following way:

            Self::emit_event(
                Self::env(),
                Event::UnlockRedeemed(UnlockRedeemed {
                    staker: user,
                    azero,
                    unlock_id: unlock_id,
                    batch_id: batch_id,
                }),
            );

[bgibers]

@0xmahdirostami -

Because the parameter names and the struct field names are exactly the same we can use shorthand initialization, and the order doesn't matter

Here's a test case you can add to drink_tests to verify:

#[derive(Debug, PartialEq)]
    struct UnlockRedeemed {
        staker: String,
        azero: u64,
        batch_id: u64,
        unlock_id: u64,
    }

    #[test]
    fn test_unlock_redeemed_initialization() {
        let staker = String::from("user1");
        let azero = 100;
        let batch_id = 1;
        let unlock_id = 10;

        let unlock1 = UnlockRedeemed {
            staker: staker.clone(),
            azero,
            unlock_id,
            batch_id,
        };

        let unlock2 = UnlockRedeemed {
            azero,
            staker: staker.clone(),
            batch_id,
            unlock_id,
        };

        println!("Unlock 1: {:?} \nUnlock 2: {:?}", unlock1, unlock2);
        assert_eq!(unlock1, unlock2);
    }

Which passes and when ran with the -- --nocapture flag outputs:

Unlock 1: UnlockRedeemed { staker: "user1", azero: 100, batch_id: 1, unlock_id: 10 } 
Unlock 2: UnlockRedeemed { staker: "user1", azero: 100, batch_id: 1, unlock_id: 10 }

Using the Field Init Shorthand when Variables and Fields Have the Same Name