[discuss] chore(connect): coinAbstractMethod #15304
Conversation
mroz22
force-pushed
the
connect-coin-abstract-method
branch
from
0af31c8
to
55450ea
Compare
| export default class BinanceGetAddress extends BinanceAbstractMethod< | ||
| 'binanceGetAddress', | ||
| Params[] | ||
| > { |
There was a problem hiding this comment.
if I give it a shorter name + remove comments, I am optimistic that this might be a line saving solution :D
|
Yeah seems good. Also when I look at GetAddress, GetPublicKey init, it seems like it's mostly the same between all methods |
| this.firmwareRange = getFirmwareRange(this.name, getMiscNetwork('BNB'), this.firmwareRange); | ||
| } | ||
|
|
||
| // firmwareRangeRequirements could be defined here instead of config.ts |
There was a problem hiding this comment.
specifically firmwareRangeRequirement for coin. basically we have now 3 levels of fw requirements
- coin
- method
- method feature
coin could be defined in this abstract method
|
hmm, ti doesn't work very well, there could be |
mroz22
force-pushed
the
connect-coin-abstract-method
branch
from
55450ea
to
d2178e1
Compare
mroz22
force-pushed
the
connect-coin-abstract-method
branch
from
d2178e1
to
2f80653
Compare
| if (!this.useDevice) { | ||
| return true; | ||
| } |
There was a problem hiding this comment.
if cardanoComposeTransaction, or any method that has useDevice: false should also inherit from a coin specific abstract method, we need to handle cases like this. I am starting to dislike it
I am exploring modularization possibilities. And it of course brings about many ideas for refactoring and improvements.
While thinking (#15291) about how to reduce the amount of data in config.ts - where specifically I don't like the information about feature support - IMHO it should be defined per coin-module.
Would it make sense to define common stuff and stuff like this in a coin specific abstract method? cc @marekrjpolak @karliatto @martykan