Skip to content

Add allowance remover#54

Open
JohnnyGoodpick wants to merge 8 commits intomainfrom
polygon-warning-modal
Open

Add allowance remover#54
JohnnyGoodpick wants to merge 8 commits intomainfrom
polygon-warning-modal

Conversation

@JohnnyGoodpick
Copy link
Collaborator

@JohnnyGoodpick JohnnyGoodpick commented Jan 6, 2023

Added a button that removes bad allowances related to the KROM_LIMIT_MANAGER on Polygon, directly on the kromatika limit page.

Seems like it's running well, one thing to check is that the proper tokenList was used when calling the button component

@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented Jan 6, 2023

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: cd35063
Status: ✅  Deploy successful!
Preview URL: https://c175ca78.interface-ctb.pages.dev
Branch Preview URL: https://polygon-warning-modal.interface-ctb.pages.dev

View logs

@JohnnyGoodpick JohnnyGoodpick changed the title base commit Add bad allowance remover Jan 6, 2023
@JohnnyGoodpick JohnnyGoodpick changed the title Add bad allowance remover Add allowance remover Jan 7, 2023
spender: string
tokenList: any
}) {
// const allTokens = useAllTokens()
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove unused code

const contractList = getContractList(tokenList, library, chainId!)
if (!contractList || contractList?.length == 0) {
console.log('unable to get contract list')
// setIsLoadingDone(true)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove unused code

{isLoadingDone && maliciousAllowanceCount == 0 ? (
<p style={{ color: 'green' }}>
{' '}
You don&apos;t have any bad allowances on this account related to Kromatika ! 🎉🎉
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

paraphrase the text to something more objective and less scary

console.log(tokenList)
const filtered = tokenList.filter((v) => {
//@ts-ignore
// console.log(v._checksummedAddress)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove unused code

//@ts-ignore
// console.log(v._checksummedAddress)
//@ts-ignore
// console.log(v.tokenInfo.name)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove unused code

})

const contractList = createERC20TokenContracts(filtered_addresses, ERC20ABI, library!)
// if (contractList.length != Object.keys(filtered).length) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove unused code

) : isLoadingDone && isMaliciousAllowancesRemoved ? (
<p style={{ color: 'green' }}> Malicious allowances have been removed! 🎉🎉</p>
) : (
<p style={{ color: 'red' }}> An error has occured ! 🎉🎉</p>
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe this case should be empty or atleast it shouldn't display an error has occured; while the txn is not approved and mined on the blockchain (while the process is ongoing), it is displaying "an error has occured"

if (Object.keys(malicious_allowances).length == 0)
console.log('You have ' + Object.keys(malicious_allowances).length + ' bad allowances')
// if (Object.keys(malicious_allowances).length == 0)
// console.log('You have ' + Object.keys(malicious_allowances).length + ' bad allowances')
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove unused code

@@ -27,6 +27,7 @@ export default function AllowanceRemover({
const [isLoading, setIsLoading] = useState(false)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove // const allTokens = useAllTokens()

@@ -41,21 +42,29 @@ export default function AllowanceRemover({
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove //setIsLoadingDone(true)

@fensa08
Copy link

fensa08 commented Jan 9, 2023

  1. change text to something more objective:
  • Do not mention bad allownace
  • For button, suggested text (or change to something similar): "Revoke all approvals for our old limit order manager contract"
  • Suggestion for text: "Successfully revoked all allowances for old limit order contract"
  1. if revoked on 1 account & account is changed afterwards, message still remains although it should not be appearing for next account/user
  2. remove warning modal
  3. add banner with the text from the modal. It should be located above both limit order components. Simple text with background color and border color.
  4. place button bellow text, but inside banner.
  5. "Error has occured" message appears before confirmation of revoke approval; It stays the whole time afterwards;
  6. Does the button revoke all approvals or just one approval ? Has this been tested ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants