Feat/external txs #850
Feat/external txs #850
Conversation
sohkai
requested review from
dizzypaty
and removed request for
bpierre and
AquiGorka
|
@dizzypaty Let's discuss the copy / security concerns of what this means when you're ready! |
There was a problem hiding this comment.
Hey @Schwartz10, sorry for taking long in reviewing this issue. Here’s a mockup with the aragonUI new style applied to the signer panel and my suggestion for the copy.
WARNING: Be aware that this is an attempt to execute a transaction on an *external contract* that has not been reviewed or audited. This means that it might behave unexpectedly. Please *be sure your trust this contract* before proceeding.
Figma assets here.
I’ve started exploring the idea of displaying a “security badge” that we could assign to contracts depending on whether these have been reviewed & audited and the outcomes of those audits (trusted, not trusted, unsafe). We’ll keep you in the loop with the direction this takes¡ 🤗
Also, we will create specific visual cues / icons to differentiate between known / unknown or external contacts. For now we can use the ‘app default’ one that you see on the design above for the compact AddressBadge.
|
@dizzypaty We can also detect if the app is calling another installed app (to separate it from an external contract, which would be a totally arbitrary contract we don't know about). One suggestion is to just change the copy slightly to indicate this, something along the lines of: |
|
Hey @sohkai this is set for your review. I'm not sure how to make the UI compatible with the latest aragon/ui components. Right now I'm using an |
There was a problem hiding this comment.
Right now I'm using an Info.Action component to render the warning text. Is that the best way?
If it's not too much trouble, it would be better to merge this against newstyle after #901 is merged, so we can use the new Tag component.
Otherwise, a couple of small and simple notes:
| } installed in this organization. You may want to double check that app's functionality before proceeding.`} | ||
| {installed | ||
| ? `Be aware that this is an attempt to execute a transaction on another app that is installed in this organization. You may want to double check that app's functionality before proceeding.` | ||
| : `Be aware that this is an attempt to execute a transaction on an *external contract* that has not been reviewed or audited. This means that it might behave unexpectedly. Please *be sure your trust this contract* before proceeding.`} |
There was a problem hiding this comment.
Unfortunately we can't use markdown in react; let's use <strong>s here :).
… update to new Info
… display it before any possible pretransactions
|
@Schwartz10 I've pushed a few commits, mostly merging it with |
…tions * origin/newstyle: MenuPanel tweaks (#933) Home redesign (#934) SignerPanel: consolidate external transaction props into intent object (#931) useLocalIdentity: handle remove case (#930) Add AppInternal to manage the layout logic of internal apps (#932) MenuPanel: adjust for new styles (#923) SignerPanel: display warning for external transactions (#850) Remove Badge and update occurrences for Tag (#901) SignerPanel: adjust for new styles (#920) Organization Settings: replace old Settings app (#896) Permissions: new style (#899) Sidepanel: redesign feedback indicator (#907) eslint: make sure curly braces are used everywhere (#924) Org switcher: new style + add FavoritesMenu (#925)
Fixes #536 and goes along with aragon/aragon.js#328
Think we could do a better job with language, but this could be a starting point that we improve as we start to see new designs and more informative copy.
Also, I'm not sure why my linter complained about
src/aragonjs-wrapper.js- i didnt touch that file?