GoodDollar / GoodServer

Backend to support the GoodDAPP
MIT License
13 stars 12 forks source link

add: topwallet and whitelist on celo #385

Closed sirpy closed 1 year ago

sirpy commented 1 year ago

Description

Support topwallet + whitelist on Celo

lgtm-com[bot] commented 1 year ago

This pull request introduces 1 alert and fixes 1 when merging 8068ad4b394a2b43c54b58e2620c652d668e90ca into 9c0102d58ebd752677bfc3484d8bc415fdcd9f6c - view on LGTM.com

new alerts:

fixed alerts:

lgtm-com[bot] commented 1 year ago

This pull request introduces 1 alert and fixes 1 when merging 7ec20ebc496a792087142d2546d8708ec1181b7d into 9c0102d58ebd752677bfc3484d8bc415fdcd9f6c - view on LGTM.com

new alerts:

fixed alerts:

lgtm-com[bot] commented 1 year ago

This pull request fixes 1 alert when merging b6f663f3337453212613bea0c49ca0797eb05926 into 9c0102d58ebd752677bfc3484d8bc415fdcd9f6c - view on LGTM.com

fixed alerts:

johnsmith-gooddollar commented 1 year ago

something wrong with the linter, need to re-check it carefully, build and test once i will finish review

lgtm-com[bot] commented 1 year ago

This pull request fixes 1 alert when merging 5a78b1cb39ad1e2295fcc8b6e31957284625ce72 into 9c0102d58ebd752677bfc3484d8bc415fdcd9f6c - view on LGTM.com

fixed alerts:

lgtm-com[bot] commented 1 year ago

This pull request fixes 3 alerts when merging 8b3963887a7aa7b8433e7cf3ec8f0f36edd14517 into 9c0102d58ebd752677bfc3484d8bc415fdcd9f6c - view on LGTM.com

fixed alerts:

lgtm-com[bot] commented 1 year ago

This pull request fixes 3 alerts when merging 3fa55149e010b1285df80b776fa2ce84455d713c into 9c0102d58ebd752677bfc3484d8bc415fdcd9f6c - view on LGTM.com

fixed alerts:

lgtm-com[bot] commented 1 year ago

This pull request fixes 3 alerts when merging 36279cb27c6cea0449ebc8254b31e150ef9cf646 into 9c0102d58ebd752677bfc3484d8bc415fdcd9f6c - view on LGTM.com

fixed alerts:

johnsmith-gooddollar commented 1 year ago

@sirpy @L03TJ3 We have issues with initializing Celo wallet

  1. there's no 'development-celo' network at '@gooddollar/goodprotocol/releases/deployment.json'. The same for QA, only prod section exists. ==> I've added CELO_FALLBACK_TO_PROD (default true) which enables using production values if nothing found on dev/qa env
  2. there's no 'AdminWallet' address in 'production-celo' section. So celo wallet unable to initialize and app crashes (because address is undefined). ===> i've added CELO_ENABLED (default false) which disables celo from everywhere. So PR could be merged without affected dev/qa instances. But to support celo, we have to add admin wallet address and dev/qa network settings to goodprotocol
sirpy commented 1 year ago

@johnsmith-gooddollar need to install the latest beta of goodprotocol, i've wrote that on the slack channel

sirpy commented 1 year ago

@johnsmith-gooddollar install the beta goodprotocol package, fix tests

johnsmith-gooddollar commented 1 year ago

@sirpy i was checking latest beta (from master branch) of the protocol package - there was still 'production-celo' only network and no 'AdminWallet' address inside.

johnsmith-gooddollar commented 1 year ago

@sirpy Also, Lewis suggested me the address '0x3De7216149f12D8f51540D9A870149560FC11bfB'. I don't checked it too, will check today. If it works i will adjust env flags a bit:

CELO_CUSTOM_NETWORK=production_celo // could be set while no env settings in contracts, only prod CELO_CUSTOM_ADMIN_ADDRESS=0x3De7216149f12D8f51540D9A870149560FC11bfB // could be set white no 'AdminWallet' address in celo contracts section

sirpy commented 1 year ago

@sirpy Also, Lewis suggested me the address '0x3De7216149f12D8f51540D9A870149560FC11bfB'. I don't checked it too, will check today. If it works i will adjust env flags a bit:

CELO_CUSTOM_NETWORK=production_celo // could be set while no env settings in contracts, only prod CELO_CUSTOM_ADMIN_ADDRESS=0x3De7216149f12D8f51540D9A870149560FC11bfB // could be set white no 'AdminWallet' address in celo contracts section

no hacks please

johnsmith-gooddollar commented 1 year ago

@sirpy it won't work without them. the latest beta also won't fix the issue - i DMed in Slack

johnsmith-gooddollar commented 1 year ago

@sirpy thanks it works now i removed all workarounds & pushed update

now will resolve conflicts then check tests

lgtm-com[bot] commented 1 year ago

This pull request fixes 3 alerts when merging a8ad9c5d9b3301e8cadb4e73b48cb2fa18a53dae into dfb487adf451695de0ae23cef99876db4abac990 - view on LGTM.com

fixed alerts:

johnsmith-gooddollar commented 1 year ago

@sirpy solved conflicts and fixed FV tests, there are few more failing at storage / addUserSteps. working on that

lgtm-com[bot] commented 1 year ago

This pull request fixes 3 alerts when merging a5f468e8ba2f9f73da120f6ff4272a261bd837b2 into dfb487adf451695de0ae23cef99876db4abac990 - view on LGTM.com

fixed alerts:

lgtm-com[bot] commented 1 year ago

This pull request fixes 3 alerts when merging 29d21c94af3b131c146f6675e7b9b8c29d06d696 into dfb487adf451695de0ae23cef99876db4abac990 - view on LGTM.com

fixed alerts:

lgtm-com[bot] commented 1 year ago

This pull request fixes 3 alerts when merging 74aef4e0dd6b1dffca036131057ac99fdd88bd60 into dfb487adf451695de0ae23cef99876db4abac990 - view on LGTM.com

fixed alerts:

johnsmith-gooddollar commented 1 year ago

@L03TJ3 @sirpy Fixed everything expect blockchain tests failing because of 'AdminWallet contract low funds' / 'Returned values aren't valid, did it run Out of Gas?' The same error happening on master branch and BC test also failing, so this looks not related to this PR changes

could you please check ? Thanks

johnsmith-gooddollar commented 1 year ago

@sirpy @L03TJ3 Also there's no test network for Celo. On AdminWallet we're using 'dapptest' / 'dapptest-mainnet', so i suppose the 'dattest-celo' should be added ? For now tests are using 'development' network (development-celo): https://github.com/GoodDollar/GoodServer/pull/385/files#diff-755919bd4c09bf52e7c7dcba502d0ecf2eca94857728bce80acd24906424aebaR5

lgtm-com[bot] commented 1 year ago

This pull request fixes 3 alerts when merging 3398556a9bcaf16f441f3b14ae2146b7b3d54f6e into dfb487adf451695de0ae23cef99876db4abac990 - view on LGTM.com

fixed alerts:

lgtm-com[bot] commented 1 year ago

This pull request fixes 3 alerts when merging 36c4d4a71cd56ba829e0a351ff27d8d0665bb07c into dfb487adf451695de0ae23cef99876db4abac990 - view on LGTM.com

fixed alerts:

lgtm-com[bot] commented 1 year ago

This pull request fixes 3 alerts when merging d7a9eba2819033313939c5a57548e5be865aeb0c into dfb487adf451695de0ae23cef99876db4abac990 - view on LGTM.com

fixed alerts:

lgtm-com[bot] commented 1 year ago

This pull request fixes 3 alerts when merging ee7a937c9d93d6772c6d43d1c547e930f6fd9e59 into 5558faf2bc55340c868217a044535835bd881842 - view on LGTM.com

fixed alerts:

lgtm-com[bot] commented 1 year ago

This pull request fixes 3 alerts when merging bd749ad7f0b1fc97aa8bd85827bf5e5027311642 into 97a01950e22c257b5a67d5908511e8df38bce3ce - view on LGTM.com

fixed alerts:

lgtm-com[bot] commented 1 year ago

This pull request fixes 3 alerts when merging 7447977681f9840acda5362840f2d57fe433185b into cc127e27f1693060603c290007c27a711b42a1a2 - view on LGTM.com

fixed alerts:

lgtm-com[bot] commented 1 year ago

This pull request fixes 3 alerts when merging a72569ad24e5137037abe56906bb2d0b6a0e240c into cc127e27f1693060603c290007c27a711b42a1a2 - view on LGTM.com

fixed alerts:

johnsmith-gooddollar commented 1 year ago

@sirpy could it be merged now ?