-
Notifications
You must be signed in to change notification settings - Fork 3
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
feat: announcement banner on deposit withdraw screens #297
base: master
Are you sure you want to change the base?
feat: announcement banner on deposit withdraw screens #297
Conversation
3c89724
to
3b5f7b9
Compare
3b5f7b9
to
184f8df
Compare
config.schema.json
Outdated
@@ -516,6 +516,61 @@ | |||
} | |||
} | |||
}, | |||
"transfer_banner": { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe can rename this field disabled_transfer_banner_config
for more clarity.
.github/markets/pr_template.md
Outdated
## TransferBanner Data Structure | ||
|Field |Type |Required |Description |Notes | | ||
|---|---|---|---|---| | ||
|`no_longer_supported_tokens` |`string[]` |false |List of tokens are no longer supported | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Please rename this field to
unsupported_tokens
- Please revise this description to
List of tokens that are no longer supported
- Please add a note stating that
The token denoms listed here **MUST** match the token denoms listed under the Carbon [Tokens API](https://api.carbon.network/carbon/coin/v1/tokens?pagination.limit=10000).
.
.github/markets/pr_template.md
Outdated
|Field |Type |Required |Description |Notes | | ||
|---|---|---|---|---| | ||
|`no_longer_supported_tokens` |`string[]` |false |List of tokens are no longer supported | | ||
|`temporary_disabled_transfer_tokens` |`object` |false |List of tokens for which deposits and withdrawals have been temporarily disabled | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Pls rename this field to
temp_disabled_transfer_tokens
- Please add a note that
The token denoms listed in this object **MUST** match the token denoms listed under the Carbon [Tokens API](https://api.carbon.network/carbon/coin/v1/tokens?pagination.limit=10000).
.github/markets/pr_template.md
Outdated
|---|---|---|---|---| | ||
|`no_longer_supported_tokens` |`string[]` |false |List of tokens are no longer supported | | ||
|`temporary_disabled_transfer_tokens` |`object` |false |List of tokens for which deposits and withdrawals have been temporarily disabled | | ||
|`temporary_disabled_bridges` |`object` |false |List of bridges for which deposits and withdrawals have been temporarily disabled | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Pls rename this field to
temp_disabled_bridges
- Please add a note that
Blockchain network listed here **MUST** match the valid chainName of the bridges listed under BridgeAll RPC call.<br /><br /> To view the values of BridgeAll RPC call, simply run
yarn get-bridges [network]on the command line. Sample for mainnet:
yarn get-bridges mainnet``
config.schema.json
Outdated
"required": [ | ||
"start", | ||
"end" | ||
], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Think instead of assigning start
and end
to empty strings, would be good to make both fields optional, then you just need to make it an empty object.
i.e.
{
"temporary_disabled_transfer_tokens": {
"token_1": {}
}
}
scripts/check_configs.ts
Outdated
@@ -426,6 +443,35 @@ function isValidMarketPromo(marketPromo: {[marketId: string]: MarketPromo}, netw | |||
return true; | |||
} | |||
|
|||
function isValidTransferBanner(transferBanner: TransferBanner, network: CarbonSDK.Network): boolean { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add a check for no_longer_supported_tokens
to make sure that all the token denoms are valid. Please refer to this code snippet to see how to check if denoms are valid.
scripts/check_configs.ts
Outdated
function isValidTransferBanner(transferBanner: TransferBanner, network: CarbonSDK.Network): boolean { | ||
const { temporary_disabled_transfer_tokens, temporary_disabled_bridges } = transferBanner; | ||
if (Object.keys(temporary_disabled_transfer_tokens).length > 0) { | ||
Object.keys(temporary_disabled_transfer_tokens).map((key) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- I see
Object.keys(temporary_disabled_transfer_tokens)
is used twice, pls store this in a variable and apply where necessary.
i.e.
const disabledTokenKeys = Object.keys(temporary_disabled_transfer_tokens)
if (disabledTokenKeys.length > 0) {
disabledTokenKeys.forEach((key) => { ... })
}
- Pls check that each denom key is valid. Please refer to this code snippet to see how to check if denoms are valid.
scripts/check_configs.ts
Outdated
const startTime = new Date(start); | ||
const endTime = new Date(end); | ||
if (endTime < startTime) { | ||
console.error(`ERROR: ${network}.json has invalid end time (${end}) is before start time (${start}) for token denom ${key}.`); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pls revise the message to ERROR: ${network}.json has an invalid end time (${end}) as it is before start time (${start}) for token denom ${key}.
scripts/check_configs.ts
Outdated
} | ||
|
||
if (Object.keys(temporary_disabled_bridges).length > 0) { | ||
Object.keys(temporary_disabled_bridges).map((key) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- I see
Object.keys(temporary_disabled_bridges)
is used twice, pls store this in a variable and apply where necessary.
i.e.
const disabledBridgesKeys = Object.keys(temporary_disabled_bridges)
if (disabledBridgesKeys.length > 0) {
disabledBridgesKeys.forEach((key) => { ... })
}
- Pls check that each bridge key is valid. Please refer to this code snippet to see how to check if the bridge names are valid.
scripts/check_configs.ts
Outdated
const startTime = new Date(start); | ||
const endTime = new Date(end); | ||
if (endTime < startTime) { | ||
console.error(`ERROR: ${network}.json has invalid end time (${end}) is before start time (${start}) for bridge address ${key}.`); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pls revise the message to ERROR: ${network}.json has an invalid end time (${end}) as it is before start time (${start}) for bridge ${key}.
Other Comments
|
No description provided.