search

brave / brave-browser

Brave browser for Android, iOS, Linux, macOS, Windows.
https://brave.com
Mozilla Public License 2.0
17.06k stars 2.23k forks source link

Stub BraveWalletServiceDelegate implementation on iOS #31395

Open darkdh opened 1 year ago

darkdh commented 1 year ago

The delegate was meant for desktop only and gets renamed to general delegate and got extended for broader use. https://github.com/brave/brave-core/commit/51b44af2e1ceaf50e86808404b28791a1ddbb6fb So that is why iOS still has stub for that.

Current codes relies on BraveWalletService::GetActiveOriginSync() such as BraveWalletService::GetChainIdForActiveOriginSync and BraveWalletService::SetChainIdForActiveOrigin won't work correctly on iOS because it will always be empty origin.

According to @kylehickinson , iOS doesn't have the capacity to get active origin from browser context like chromium does (TabStripModel)

We should clearly mark some methods in the delegate are not iOS compatible like IsExternalWalletInstalled, IsExternalWalletInitialized, GetImportInfoFromExternalWallet, GetActiveOrigin and ClearWalletUIStoragePartition through build flag. Or the whole delegate not iOS compatible.

cc @StephenHeaps to double check if selected network per origin works as expected on iOS and if iOS needs implement AddPermission, HasPermission, ResetPermission and IsPermissionDenied for dapps permissions

cc @supermassive that iOS is currently not using AddPermission, HasPermission, ResetPermission and IsPermissionDenied so we can remove passing origin back and forth if they don't plan to implement them.

darkdh commented 1 year ago

Just got confirmation from @StephenHeaps, iOS doesn't use GetChainIdForActiveOrigin nor SetChainIdForActiveOrigin from BraveWalletService, they interact with JsonRpcService directly for instance GetNetwork