-
Notifications
You must be signed in to change notification settings - Fork 986
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: integrate base chain #21876
feat: integrate base chain #21876
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -30,13 +30,16 @@ | |
|
||
(defn networks | ||
[values theme] | ||
(let [{:keys [ethereum optimism arbitrum]} values | ||
show-optimism? (and optimism | ||
(or (pos? (:amount optimism)) | ||
(= (:amount optimism) "<0.01"))) | ||
show-arbitrum? (and arbitrum | ||
(or (pos? (:amount arbitrum)) | ||
(= (:amount arbitrum) "<0.01")))] | ||
(let [{:keys [ethereum optimism arbitrum base]} values | ||
show-optimism? (and optimism | ||
(or (pos? (:amount optimism)) | ||
(= (:amount optimism) "<0.01"))) | ||
show-arbitrum? (and arbitrum | ||
(or (pos? (:amount arbitrum)) | ||
(= (:amount arbitrum) "<0.01"))) | ||
show-base? (and base | ||
(or (pos? (:amount base)) | ||
(= (:amount base) "<0.01")))] | ||
[rn/view | ||
{:style style/networks-container | ||
:accessibility-label :networks} | ||
|
@@ -56,6 +59,11 @@ | |
[network-amount | ||
{:network :arbitrum | ||
:amount (str (:amount arbitrum) " " (or (:token-symbol arbitrum) "ARB")) | ||
:theme theme}]) | ||
(when show-base? | ||
[network-amount | ||
{:network :base | ||
:amount (str (:amount base) " " (or (:token-symbol base) "ETH")) | ||
:theme theme}])])) | ||
Comment on lines
+64
to
67
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The |
||
|
||
(defn- view-internal | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -135,7 +135,8 @@ | |
(def ^:private network-priority-score | ||
{:ethereum 1 | ||
:optimism 2 | ||
:arbitrum 3}) | ||
:arbitrum 3 | ||
:base 4}) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can move base to rank 2 |
||
|
||
(defn reset-loading-network-amounts-to-zero | ||
[network-amounts] | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,7 +13,8 @@ | |
:bsc-testnet {:id BSC-testnet-chain-id | ||
:name "BSC testnet"} | ||
:arbitrum {:id 42161 :name "Arbitrum"} | ||
:optimism {:id 10 :name "Optimism"}}) | ||
:optimism {:id 10 :name "Optimism"} | ||
:base {:id 8453 :name "Base"}}) | ||
Comment on lines
15
to
+17
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we have these values as constants already There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These data need to be cleaned as it was used by old wallet code |
||
|
||
(defn chain-id->chain-keyword | ||
[i] | ||
|
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.
It's strange we're adding changes to quo components when adding a new chain, IMO it should be more generic, but it's a larger issue with quo.
For now though, ideally we would want to minimise "chain-specific" code to make it easier to add other ones in the future. This check could be extracted into a function or a separate component, since the operation is the same for each chain.
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 totally agree @clauxx, I think all these checks are not needed anymore as we are showing only one chain at a time, but I'd prefer to refactor in a follow-up as this would be out of scope of this PR.