Skip to content

WIP: Merge token accounts#64

Open
mocolicious wants to merge 10 commits intomasterfrom
merge-token-accounts
Open

WIP: Merge token accounts#64
mocolicious wants to merge 10 commits intomasterfrom
merge-token-accounts

Conversation

@mocolicious
Copy link
Copy Markdown
Member

No description provided.

}

var dest = existingAccounts[0].key;
List<Instruction> instructions = List.empty();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This empty list is not growable by default. Just declare using <Instruction>[], as Dart recommendation.

https://api.dart.dev/stable/2.14.4/dart-core/List/List.empty.html

instructions.add(MemoProgramBase64EncodedMemo.fromBytes((memo as KinBinaryMemo).encode()).instruction!);
}

instructions.add(createAssocAccount.instruction!);
Copy link
Copy Markdown
Contributor

@gmpassos gmpassos Nov 17, 2021

Choose a reason for hiding this comment

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

Here you are adding to another growable list.

).instruction!);

dest = createAssocAccount.addr;
signers.add(signer);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Adding to a not growable list.


var dest = existingAccounts[0].key;
List<Instruction> instructions = List.empty();
List<PrivateKey> signers = List.empty();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Same issue here. Just use <PrivateKey>[] to declare the list. Is the most portable and efficient way, since Dart will choose the most efficient implementation for the platform and allow more optimizations.

}
}

existing:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Do not declare a loop label if there's not a loop inside a loop.

import 'dart:ffi';
import 'dart:typed_data';

import 'package:crypto/crypto.dart';
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Unecessary import

AssociatedTokenProgramCreateAssociatedTokenAccount(
this.subsidizer, this.wallet, this.mint);

late final addr = (AssociatedTokenProgram().getAssociatedAccount(wallet, mint) as PublicKey);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

No need of ( and )


Instruction? _instruction;

Instruction? get instruction {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

use the ??= operator

Instruction? _instruction;

Instruction? get instruction {
if(_instruction == null) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

you are not returning the _instruction.

Use the ??= operator

var publicKeys = accounts.tokenAccountInfos;

return new KinServiceResponse(KinServiceResponseType.ok, publicKeys);
return new KinServiceResponse(KinServiceResponseType.ok, publicKeys.map((e) => KinTokenAccountInfo(PublicKey(e.accountId.toString()), KinAmount.fromInt(e.balance.toInt()), PublicKey(e.closeAuthority.toString()))).toList());
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

separate in 2 lines. Too many things in the same statement.

Copy link
Copy Markdown
Member Author

@mocolicious mocolicious left a comment

Choose a reason for hiding this comment

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

finished changes

@mocolicious
Copy link
Copy Markdown
Member Author

Review complete, pending testing

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.

Multiple token accounts created for wallet, whilst not being able to transact from any other token account.

2 participants