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.

Sign up for GitHub

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

Jump to bottom

sdk: allow delegates in single client #432

Merged
merged 21 commits into from
Apr 27, 2023
Merged

sdk: allow delegates in single client #432

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
4c02ea4
sdk: updates for delegate wip
lowkeynicc Apr 11, 2023
978fd42
save
lowkeynicc Apr 11, 2023
2406bad
wip
lowkeynicc Apr 12, 2023
bd70516
updates
lowkeynicc Apr 12, 2023
8ef24c1
updates
lowkeynicc Apr 12, 2023
7a61cc0
feedback
lowkeynicc Apr 14, 2023
25438f3
updates
lowkeynicc Apr 14, 2023
96ec680
merge
lowkeynicc Apr 17, 2023
b19cdd1
merge conflict
lowkeynicc Apr 17, 2023
e931f2e
updates
lowkeynicc Apr 17, 2023
bf06214
sdk: fix userStats account
lowkeynicc Apr 19, 2023
615d220
add authoritySubaccountMap
lowkeynicc Apr 25, 2023
24d651b
fix
lowkeynicc Apr 25, 2023
aee3d1b
disallow subaccountmap and includedelegates
lowkeynicc Apr 25, 2023
1200b45
more feedback
lowkeynicc Apr 25, 2023
a706d73
update to work for user with only delegate accounts
lowkeynicc Apr 26, 2023
68c1ac1
updates to authoritysubaccountmap
lowkeynicc Apr 26, 2023
791f2d4
update changelog + fix default to delegate acct
lowkeynicc Apr 26, 2023
91c4bc6
merge
lowkeynicc Apr 26, 2023
9d74c0c
fix prettier
lowkeynicc Apr 26, 2023
dca79b5
add skipAddUsers flag
lowkeynicc Apr 27, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
172 changes: 111 additions & 61 deletions sdk/src/driftClient.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,9 +117,10 @@ export class DriftClient {
public program: Program;
provider: AnchorProvider;
opts?: ConfirmOptions;
users = new Map<number, User>();
users = new Map<string, User>();
userStats?: UserStats;
activeSubAccountId: number;
activeAuthority: PublicKey;
userAccountSubscriptionConfig: UserSubscriptionConfig;
accountSubscriber: DriftClientAccountSubscriber;
eventEmitter: StrictEventEmitter<EventEmitter, DriftClientAccountEvents>;
Expand Down Expand Up @@ -154,8 +155,8 @@ export class DriftClient {
);

this.authority = config.authority ?? this.wallet.publicKey;
const subAccountIds = config.subAccountIds ?? [0];
this.activeSubAccountId = config.activeSubAccountId ?? subAccountIds[0];
this.activeSubAccountId = config.activeSubAccountId;
Copy link
Member

Choose a reason for hiding this comment

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

why'd you get rid of the ?? [0]?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this way it will start undefined if not specified, then the call to addUsers will know no default is set and use the first user

Copy link
Member

Choose a reason for hiding this comment

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

i think this might break some backwards compatibility since not every user will call addUsers

this.activeAuthority = this.authority;
this.userAccountSubscriptionConfig =
config.accountSubscription?.type === 'polling'
? {
Expand All @@ -165,7 +166,14 @@ export class DriftClient {
: {
type: 'websocket',
};
this.createUsers(subAccountIds, this.userAccountSubscriptionConfig);

// load all or some users and add them to the driftClient
this.addAllUsers(
Copy link
Member

Choose a reason for hiding this comment

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

i dont think we want to do any async calls in the constructor, thats why the createUsers call didn't call subscribe before

Copy link
Member

Choose a reason for hiding this comment

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

maybe we have a new DriftClientConfig Param which is authoritySubaccountMap which allows you to specify the authority & subaccount pairs

config.subAccountIds,
!this.activeSubAccountId,
config.includeDelegates
);

if (config.userStats) {
this.userStats = new UserStats({
driftClient: this,
Expand Down Expand Up @@ -227,23 +235,18 @@ export class DriftClient {
);
}

createUsers(
subAccountIds: number[],
accountSubscriptionConfig: UserSubscriptionConfig
): void {
for (const subAccountId of subAccountIds) {
const user = this.createUser(subAccountId, accountSubscriptionConfig);
this.users.set(subAccountId, user);
}
public getUserMapKey(subAccountId: number, authority: PublicKey): string {
return `${subAccountId}_${authority.toString()}`;
}

createUser(
subAccountId: number,
accountSubscriptionConfig: UserSubscriptionConfig
accountSubscriptionConfig: UserSubscriptionConfig,
authority?: PublicKey
): User {
const userAccountPublicKey = getUserAccountPublicKeySync(
this.program.programId,
this.authority,
authority ?? this.authority,
subAccountId
);

Expand Down Expand Up @@ -427,11 +430,13 @@ export class DriftClient {
* @param newWallet
* @param subAccountIds
* @param activeSubAccountId
* @param includeDelegates
*/
public async updateWallet(
newWallet: IWallet,
subAccountIds = [0],
activeSubAccountId = 0
subAccountIds?: number[],
activeSubAccountId?: number,
includeDelegates?: boolean
): Promise<void> {
const newProvider = new AnchorProvider(
this.connection,
Expand All @@ -446,11 +451,19 @@ export class DriftClient {

// Update provider for txSender with new wallet details
this.txSender.provider = newProvider;

this.wallet = newWallet;
this.provider = newProvider;
this.program = newProgram;
this.authority = newWallet.publicKey;
this.activeSubAccountId = activeSubAccountId;
this.userStatsAccountPublicKey = undefined;

this.users.clear();
await this.addAllUsers(
Copy link
Member

Choose a reason for hiding this comment

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

why are you adding the users before the unsubscribe? feel like this is backwards?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

mistake, not sure how it wasn't breaking anything

subAccountIds,
!this.activeSubAccountId,
includeDelegates
);

if (this.isSubscribed) {
await Promise.all(this.unsubscribeUsers());
Expand All @@ -460,49 +473,71 @@ export class DriftClient {

this.userStats = new UserStats({
driftClient: this,
userStatsAccountPublicKey: getUserStatsAccountPublicKey(
this.program.programId,
this.authority
),
userStatsAccountPublicKey: this.getUserStatsAccountPublicKey(),
accountSubscription: this.userAccountSubscriptionConfig,
});
}
}
this.users.clear();
this.createUsers(subAccountIds, this.userAccountSubscriptionConfig);
if (this.isSubscribed) {
await Promise.all(this.subscribeUsers());

if (this.userStats) {
await this.userStats.subscribe();
}
}

this.activeSubAccountId = activeSubAccountId;
this.userStatsAccountPublicKey = undefined;
}

public switchActiveUser(subAccountId: number) {
public switchActiveUser(subAccountId: number, authority?: PublicKey) {
this.activeSubAccountId = subAccountId;
this.activeAuthority = authority ?? this.authority;
}

public async addUser(subAccountId: number): Promise<void> {
if (this.users.has(subAccountId)) {
public async addUser(
subAccountId: number,
authority?: PublicKey
): Promise<void> {
authority = authority ?? this.authority;
const userKey = this.getUserMapKey(subAccountId, authority);

if (this.users.has(userKey)) {
return;
}

const user = this.createUser(
subAccountId,
this.userAccountSubscriptionConfig
this.userAccountSubscriptionConfig,
authority
);
await user.subscribe();
this.users.set(subAccountId, user);
this.users.set(userKey, user);
}

public async addAllUsers(): Promise<void> {
const userAccounts = await this.getUserAccountsForAuthority(this.authority);
public async addAllUsers(
subAccountIds?: number[],
Copy link
Member

Choose a reason for hiding this comment

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

is this just for the constructor? would be nice if we didn't need to add this as it'd make the function less confusing

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 to allow them to only subscribe to a subset of subaccount ids

setFirstActive?: boolean,
Copy link
Member

Choose a reason for hiding this comment

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

when do you set this true versus false, trying to understand when we'd use it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

at the moment if the user passes a subaccount id to the constructor or updateWallet it defaults the user to that, if not then this flag is true and it defaults to the first user

Copy link
Member

Choose a reason for hiding this comment

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

i feel like the constructor always needs to set the activeSubAccount for backwards compatability

includeDelegates?: boolean
Copy link
Member

Choose a reason for hiding this comment

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

couldn't you just check that if the this.authority == wallet.pubkey to decide if the user has delegate set?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

not sure what you mean by this, this flag indicates we want to load any/all of the accounts delegated to an authority as part of their subscribed driftclient users

Copy link
Member

Choose a reason for hiding this comment

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

yeah i don't think my comment made sense, nvm

): Promise<void> {
let userAccounts =
(await this.getUserAccountsForAuthority(this.authority)) ?? [];
if (subAccountIds) {
userAccounts = userAccounts.filter((userAccount) =>
subAccountIds.includes(userAccount.subAccountId)
);
}

for (const userAccount of userAccounts) {
await this.addUser(userAccount.subAccountId);
this.addUser(userAccount.subAccountId, this.authority);
Copy link
Member

Choose a reason for hiding this comment

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

not having await for these addUser will lead to race conditions?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

didn't have any race condition issues with it and tested a lot, wanted to make it faster, will add back

}

let delegatedAccounts = [];
if (includeDelegates) {
delegatedAccounts = await this.getUserAccountsForDelegate(this.authority);
for (const delegatedAccount of delegatedAccounts) {
this.addUser(delegatedAccount.subAccountId, delegatedAccount.authority);
}
}

if (setFirstActive) {
const firstUser = userAccounts[0] ?? delegatedAccounts[0];
this.switchActiveUser(
firstUser?.subAccountId ?? 0,
firstUser?.authority ?? this.authority
);
}
}

Expand Down Expand Up @@ -689,7 +724,7 @@ export class DriftClient {
subAccountId
);

await this.addUser(subAccountId);
await this.addUser(subAccountId, this.wallet.publicKey);
const remainingAccounts = this.getRemainingAccounts({
userAccounts: [this.getUserAccount(subAccountId)],
});
Expand Down Expand Up @@ -761,9 +796,9 @@ export class DriftClient {
},
]);

return programAccounts.map(
(programAccount) => programAccount.account as UserAccount
);
return programAccounts
.map((programAccount) => programAccount.account as UserAccount)
.sort((a, b) => a.subAccountId - b.subAccountId);
}

public async getUserAccountsAndAddressesForAuthority(
Expand Down Expand Up @@ -797,9 +832,9 @@ export class DriftClient {
},
]);

return programAccounts.map(
(programAccount) => programAccount.account as UserAccount
);
return programAccounts
.map((programAccount) => programAccount.account as UserAccount)
.sort((a, b) => a.subAccountId - b.subAccountId);
}

public async getReferredUserStatsAccountsByReferrer(
Expand Down Expand Up @@ -863,18 +898,22 @@ export class DriftClient {
this.opts
);

await this.users.get(subAccountId)?.unsubscribe();
this.users.delete(subAccountId);
const userMapKey = this.getUserMapKey(subAccountId, this.wallet.publicKey);
await this.users.get(userMapKey)?.unsubscribe();
this.users.delete(userMapKey);

return txSig;
}

public getUser(subAccountId?: number): User {
public getUser(subAccountId?: number, authority?: PublicKey): User {
subAccountId = subAccountId ?? this.activeSubAccountId;
if (!this.users.has(subAccountId)) {
authority = authority ?? this.activeAuthority;
const userMapKey = this.getUserMapKey(subAccountId, authority);

if (!this.users.has(userMapKey)) {
throw new Error(`Clearing House has no user for user id ${subAccountId}`);
}
return this.users.get(subAccountId);
return this.users.get(userMapKey);
}

public getUsers(): User[] {
Expand Down Expand Up @@ -911,12 +950,18 @@ export class DriftClient {
return this.userStatsAccountPublicKey;
}

public async getUserAccountPublicKey(): Promise<PublicKey> {
return this.getUser().userAccountPublicKey;
public async getUserAccountPublicKey(
subAccountId?: number,
authority?: PublicKey
): Promise<PublicKey> {
return this.getUser(subAccountId, authority).userAccountPublicKey;
}

public getUserAccount(subAccountId?: number): UserAccount | undefined {
return this.getUser(subAccountId).getUserAccount();
public getUserAccount(
subAccountId?: number,
authority?: PublicKey
): UserAccount | undefined {
return this.getUser(subAccountId, authority).getUserAccount();
}

/**
Expand Down Expand Up @@ -1777,9 +1822,14 @@ export class DriftClient {
);

let remainingAccounts;
if (this.users.has(fromSubAccountId)) {

const userMapKey = this.getUserMapKey(
fromSubAccountId,
this.wallet.publicKey
);
if (this.users.has(userMapKey)) {
remainingAccounts = this.getRemainingAccounts({
userAccounts: [this.users.get(fromSubAccountId).getUserAccount()],
userAccounts: [this.users.get(userMapKey).getUserAccount()],
useMarketLastSlotCache: true,
writableSpotMarketIndexes: [marketIndex],
});
Expand Down Expand Up @@ -2109,8 +2159,8 @@ export class DriftClient {
marketOrderTx.recentBlockhash = currentBlockHash;
fillTx.recentBlockhash = currentBlockHash;

marketOrderTx.feePayer = userAccount.authority;
fillTx.feePayer = userAccount.authority;
marketOrderTx.feePayer = this.authority;
fillTx.feePayer = this.authority;

const [signedMarketOrderTx, signedFillTx] =
await this.provider.wallet.signAllTransactions([marketOrderTx, fillTx]);
Expand Down Expand Up @@ -2897,7 +2947,7 @@ export class DriftClient {
user: UserAccount,
txParams?: TxParams
): Promise<TransactionSignature> {
const { txSig } = await this.txSender.send(
const { txSig } = await this.sendTransaction(
wrapInTx(
await this.getForceCancelOrdersIx(userAccountPublicKey, user),
txParams?.computeUnits,
Expand Down Expand Up @@ -2936,7 +2986,7 @@ export class DriftClient {
user: UserAccount,
txParams?: TxParams
): Promise<TransactionSignature> {
const { txSig } = await this.txSender.send(
const { txSig } = await this.sendTransaction(
wrapInTx(
await this.getUpdateUserIdleIx(userAccountPublicKey, user),
txParams?.computeUnits,
Expand Down
1 change: 1 addition & 0 deletions sdk/src/driftClientConfig.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ export type DriftClientConfig = {
env?: DriftEnv;
userStats?: boolean;
authority?: PublicKey; // explicitly pass an authority if signer is delegate
includeDelegates?: boolean; // flag for whether to load delegate accounts as well
};

export type DriftClientSubscriptionConfig =
Expand Down
2 changes: 1 addition & 1 deletion sdk/src/tx/retryTxSender.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ export class RetryTxSender implements TxSender {
opts = this.provider.opts;
}

const signedTx = preSigned
const signedTx = preSigned && tx.feePayer
Copy link
Member

Choose a reason for hiding this comment

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

if it's preSigned, why would it not have a feePayer? seems like there is an issue upstream?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no issue, just sort of a redundancy check but i can remove

? tx
: await this.prepareTx(tx, additionalSigners, opts);

Expand Down