-
Notifications
You must be signed in to change notification settings - Fork 17
Improve about screen #411
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
Improve about screen #411
Conversation
WalkthroughThe MostroInstance data model is extended with two new fields: Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/features/mostro/mostro_instance.dart (1)
23-48: Unsafe parsing inmaxOrdersPerResponsegetter will crash if tag is missing.The factory method
MostroInstance.fromEvent()correctly passes both new constructor arguments. However, line 106 usesint.parse(_getTagValue('max_orders_per_response'))which throwsFormatExceptionif the tag is missing, since_getTagValue()returns'Tag: max_orders_per_response not found'as a fallback string (line 80). Use safe parsing with a fallback value:int get maxOrdersPerResponse { final value = _getTagValue('max_orders_per_response'); return value.startsWith('Tag:') ? 0 : int.parse(value); }Or wrap with try-catch at the call site in
fromEvent.
|
Hi @Catrya, should the information (displayed on the About screen) be updated when the Mostro instance is changed? While testing, I switched to my local instance, but the info on the About screen didn’t change. It continued showing the old data. My local mostro node only accepts a few currencies.
|
|
@BraCR10 tu mostro está actualizado y publica los eventos de info con la kind 38385? debes esperar un momento despues de cambiar de instancia para que esa pantalla se actualice, fijate que todavia sale el mostro por defecto |
|
@Catrya listo funciona super bien , my bad. |
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.
De momento solo tengo esta observación
| String get supportedNetworks => _getTagValue('lnd_networks'); | ||
| String get lndNodeUri => _getTagValue('lnd_uris'); | ||
| String get fiatCurrenciesAccepted => _getTagValue('fiat_currencies_accepted'); | ||
| int get maxOrdersPerResponse => int.parse(_getTagValue('max_orders_per_response')); |
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.
_getTagValue() might return a not found message, This could throw an error if the parameter is missing, couldn't it?
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.
Sí, pero el evento tiene que tener esas tags, no deberían faltar, _getTagValue se ha usado para todas las demás siempre porque se espera que estén
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.
Listo, gracias por la aclaración
|
LGTM |
grunch
left a comment
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.
utACK


orderExpirationExplanation,mostroPublicKeyExplanation,expirationHoursandsupportedChainsExplanationSummary by CodeRabbit
New Features
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.