Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

added $GRND token w/contract info on Optimism, Polygon, and Ethereum #769

Closed
wants to merge 6 commits into from

Conversation

ipfsnut
Copy link

@ipfsnut ipfsnut commented Apr 7, 2024

Adding $GRND token to Superchain Token List.

@ipfsnut ipfsnut requested a review from a team as a code owner April 7, 2024 23:45
wbnns
wbnns previously approved these changes Apr 26, 2024
Copy link
Collaborator

@wbnns wbnns left a comment

Choose a reason for hiding this comment

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

ACK

Copy link
Contributor

@hamdiallam hamdiallam left a comment

Choose a reason for hiding this comment

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

Few comments, thank you!

.gitpod.yml Outdated Show resolved Hide resolved
data/GRND/data.json Show resolved Hide resolved
Fixed a formatting error and added information.
@mergify mergify bot dismissed hamdiallam’s stale review April 30, 2024 17:29

Pull request has been modified.

maybe this time it'll stay gone?
"optimism": {
"address": "0x337919Bc5737D046Dd4b559D2663aF7B0948EfE6"
},
"polygon": {
Copy link
Contributor

Choose a reason for hiding this comment

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

can you remove this polygon entry? Polygon is not apart of the superchain

Please run the validation script locally and ensure it's passing so that this feedback loop is a but faster

Copy link
Author

Choose a reason for hiding this comment

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

Polygon removed. We should be good to go!

Copy link
Author

Choose a reason for hiding this comment

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

I'm not sure where the validation script is - not to waste too much of your time, but if there's a link to it I am happy to run it. I'm working on doing this for $PAGE as well, and using the Base bot for the first time, so just learning this process.

Copy link
Contributor

Choose a reason for hiding this comment

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

nw! this is good signal to get our readme updated. Noticed the section about validation doesn't specify the validation command. I'll get that updated

Copy link
Contributor

Choose a reason for hiding this comment

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

Few more things to fix

2024-05-01T03:08:59.5147639Z error: GRND on chain ethereum token 0x9816082D089faD585B0735dcaf15147bBB9A4C4d has incorrect symbol
2024-05-01T03:08:59.5149563Z error: GRND on chain ethereum token 0x9816082D089faD585B0735dcaf15147bBB9A4C4d has incorrect name. Got GRND
2024-05-01T03:08:59.5151770Z error: GRND on chain optimism token 0x337919Bc5737D046Dd4b559D2663aF7B0948EfE6 has incorrect symbol
2024-05-01T03:08:59.5153278Z error: GRND on chain optimism token 0x337919Bc5737D046Dd4b559D2663aF7B0948EfE6 has incorrect name. Got GRND
2024-05-01T03:08:59.5154525Z error: GRND on chain base token 0x7E00780C65D0291E5210fC208ABce0C5A966d538 has incorrect symbol
2024-05-01T03:08:59.5155646Z error: GRND on chain base token 0x7E00780C65D0291E5210fC208ABce0C5A966d538 has incorrect name. Got GRND

The data fields need to match what's onchain, with overrides set if that's not possible

Copy link
Author

Choose a reason for hiding this comment

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

Got it. I had left in the templated Name and Symbol fields at the top, as a result of my effort to locate the formatting error. Setting those to Name and Symbol respectively did not work well with the token named GRND with a symbol of GRND. My bad! We should be good now though.

Polygon removed.
@mergify mergify bot dismissed hamdiallam’s stale review May 1, 2024 00:47

Pull request has been modified.

Found the mistake. Template artifact. Resolved now.
Copy link
Collaborator

@wbnns wbnns left a comment

Choose a reason for hiding this comment

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

@ipfsnut

Thanks for the updates! We're still seeing this when running the validator:

error: GRND on chain ethereum token 0x9816082D089faD585B0735dcaf15147bBB9A4C4d has incorrect symbol
warning: GRND on chain ethereum token 0x9816082D089faD585B0735dcaf15147bBB9A4C4d not found on CoinGecko token list

@wbnns wbnns added the base label May 3, 2024
@wbnns wbnns self-assigned this May 3, 2024
@wbnns
Copy link
Collaborator

wbnns commented May 15, 2024

Since it's been a while and we haven't heard back, closing this for now. Please let us know if you'd like to pursue the additional changes to resolve the outstanding issues on the PR and we can reopen. 👍

@wbnns wbnns closed this May 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants