hats-finance / Most--Aleph-Zero-Bridge-0xab7c1d45ae21e7133574746b2985c58e0ae2e61d

Aleph Zero bridge to Ethereum
Apache License 2.0
0 stars 1 forks source link

Incorrect `event` emission in `accept_ownership()` function #39

Open hats-bug-reporter[bot] opened 5 months ago

hats-bug-reporter[bot] commented 5 months ago

Github username: -- Twitter username: -- Submission hash (on-chain): 0x5b76bf3ce3712f183e5c450e750e4ad04bf493695b5a110f5876066a0943c45d Severity: minor

Description: Description\

Affected code:

https://github.com/Cardinal-Cryptography/most/blob/a67f7a523f4257415ca74ba8d9a27dde3b313193/azero/contracts/token/lib.rs#L285

ink based contracts has employed 2 step ownership transfer pattern. The current owner calls transfer_ownership() function to set the pending owner and later pending owner calls accept_ownership() function to get the ownership of contract.

        #[ink(message)]
        fn accept_ownership(&mut self) -> Ownable2StepResult<()> {
            let new_owner = self.env().caller();
            self.ownable_data.accept_ownership(new_owner)?;
            self.env()
@>              .emit_event(TransferOwnershipInitiated { new_owner });
            Ok(())
        }

The issue here is that, the incorrect event is emitted when the ownership is accepted by pending owner which creates misunderstanding and errors when checking events. When the event will be queried for accept_ownership(), it will give incorrect details. On understanding why correct events are needed, Please refer below issue:

https://github.com/hats-finance/Most--Aleph-Zero-Bridge-0xab7c1d45ae21e7133574746b2985c58e0ae2e61d/issues/2

POC\ 1) The pending owner calls accept_ownership() function and the event is emitted as TransferOwnershipInitiated. 2) This will clearly misguide and will create misunderstanding about the event emission to users. It will spread wrong information in the name of transparency. 3) The actual event should have been emitted is TransferOwnershipAccepted which specifically mentions pending owner has accepted contract ownership.

Recommendation to fix\

        #[ink(message)]
        fn accept_ownership(&mut self) -> Ownable2StepResult<()> {
            let new_owner = self.env().caller();
            self.ownable_data.accept_ownership(new_owner)?;
-            self.env()
-              .emit_event(TransferOwnershipInitiated { new_owner });
+            self.env()
+              .emit_event(TransferOwnershipAccepted { new_owner });
            Ok(())
        }
krzysztofziobro commented 4 months ago

Valid submission - minor