Skip to content
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

litecoin mweb support #1455

Merged
merged 253 commits into from
Sep 28, 2024
Merged

litecoin mweb support #1455

merged 253 commits into from
Sep 28, 2024

Conversation

fossephate
Copy link
Contributor

Issue Number (if Applicable): Fixes #

Description

Please include a summary of the changes and which issue is fixed / feature is added.

Pull Request - Checklist

  • Initial Manual Tests Passed
  • Double check modified code and verify it with the feature/task requirements
  • Format code
  • Look for code duplication
  • Clear naming for variables and methods

@fossephate fossephate marked this pull request as draft May 17, 2024 07:07
lib/view_model/send/send_view_model.dart Show resolved Hide resolved
cw_bitcoin/lib/litecoin_wallet_addresses.dart Show resolved Hide resolved
lib/src/screens/receive/widgets/address_list.dart Outdated Show resolved Hide resolved
lib/view_model/dashboard/balance_view_model.dart Outdated Show resolved Hide resolved
lib/view_model/dashboard/balance_view_model.dart Outdated Show resolved Hide resolved
Comment on lines 325 to 333
_stuckSyncTimer = Timer.periodic(const Duration(seconds: 10), (timer) async {
if (syncStatus is! SyncingSyncStatus) return;
if (syncStatus.progress() > 0.98) return; // don't check if we're close to synced
lastFewProgresses.add(syncStatus.progress());
if (lastFewProgresses.length < 10) return;
// limit list size to 10:
while (lastFewProgresses.length > 10) {
lastFewProgresses.removeAt(0);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

10 seconds and you need 10 to trigger the resync, so 100 seconds, isn't that a bit long to just restart synchronization?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's intentionally long as it was triggering unnecessarily, but it's more meant so that background sync doesn't get stuck forever more than anything

we can probably just move this code into the background sync branch so that it applies to all wallet types, since it doesn't do much for the foreground state anyways

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed in the latest commit (and added to the mweb-bg-sync-2 branch)

lib/core/sync_status_title.dart Outdated Show resolved Hide resolved
Comment on lines 472 to 476
// if our address isn't in the inputs, update the txCount:
final inputAddresses = tx.inputAddresses ?? [];
if (!inputAddresses.contains(utxo.address)) {
addressRecord.txCount++;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't we update the count anyway?

cw_bitcoin/lib/litecoin_wallet.dart Outdated Show resolved Hide resolved
Comment on lines +593 to +595
tx.isPending = false;
await transactionHistory.save();
return true;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what about this too?

Comment on lines +886 to +905
// check if the transaction doesn't contain any mweb inputs or outputs:
final transactionCredentials = credentials as BitcoinTransactionCredentials;

bool hasMwebInput = false;
bool hasMwebOutput = false;

for (final output in transactionCredentials.outputs) {
if (output.extractedAddress?.toLowerCase().contains("mweb") ?? false) {
hasMwebOutput = true;
break;
}
}

if (tx2.mwebBytes != null && tx2.mwebBytes!.isNotEmpty) {
hasMwebInput = true;
}

if (!hasMwebInput && !hasMwebOutput) {
return tx;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Enhancement:
would have been better to do it right before/after the if (!mwebEnabled) check, so we don't await some functions that we don't need

also what about the TODO you added regarding type casting BitcoinScriptOutput?
it should work, otherwise the compiler would have flagged it as non supported type

@OmarHatem28 OmarHatem28 merged commit 62e0c2a into main Sep 28, 2024
2 checks passed
@OmarHatem28 OmarHatem28 deleted the mweb branch September 28, 2024 02:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants