-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Fix/sol rpc get account info #4244
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
base: master
Are you sure you want to change the base?
Changes from all commits
3d3cdc1
f7ffdf0
c3c2157
7782954
9825a83
f5ab44e
93d6891
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -963,28 +963,21 @@ export class SolRpc { | |
| * @returns | ||
| */ | ||
| async getAccountInfo({ address, maxDepth }) { | ||
| try { | ||
| const accountInfoResponse = await this.rpc.getAccountInfo(address).send(); | ||
|
|
||
| const lamports = accountInfoResponse.value ? Number(accountInfoResponse.value.lamports) : 0; | ||
| let effectiveMaxDepth; | ||
| if (maxDepth === -1) { | ||
| effectiveMaxDepth = Infinity; | ||
| } else if (typeof maxDepth === 'number' && maxDepth >= 1) { | ||
| effectiveMaxDepth = maxDepth; | ||
| } else { | ||
| effectiveMaxDepth = 0; | ||
| } | ||
| const atas = await this.getTokenAccountsByOwner({ address, skipExistenceCheck: true, maxDepth: effectiveMaxDepth }); | ||
| return { lamports, atas }; | ||
| } catch (err) { | ||
| const errMsg = err.message.toLowerCase(); | ||
| if (SolKit.isSolanaError(err) && errMsg.includes('json-rpc') && errMsg.includes('should be less than 128 bytes')) { | ||
| // This message can occur when getAccountInfo is called with an SPL address instead of a SOL address | ||
| throw new Error(SOL_ERROR_MESSAGES.ATA_ADD_SENT_INSTEAD_OF_SOL_ADD); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ATA_ADD_SENT_INSTEAD_OF_SOL_ADD has no references left after this PR, the throw and the test that used it are both gone. error_messages.js:9 can be deleted, unless we end up keeping the throw per the other comment. |
||
| } | ||
| throw err; | ||
| const accountInfoResponse = await this.rpc | ||
| .getAccountInfo(address, { encoding: 'base64' }) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nothing here reads .data, but base64 drops the old base58 size limit that was keeping these responses small. A big account now sends its whole data payload on every call. Adding |
||
| .send(); | ||
|
MichaelAJay marked this conversation as resolved.
|
||
|
|
||
| const lamports = accountInfoResponse.value ? Number(accountInfoResponse.value.lamports) : 0; | ||
| let effectiveMaxDepth; | ||
| if (maxDepth === -1) { | ||
| effectiveMaxDepth = Infinity; | ||
| } else if (typeof maxDepth === 'number' && maxDepth >= 1) { | ||
| effectiveMaxDepth = maxDepth; | ||
| } else { | ||
| effectiveMaxDepth = 0; | ||
| } | ||
| const atas = await this.getTokenAccountsByOwner({ address, skipExistenceCheck: true, maxDepth: effectiveMaxDepth }); | ||
| return { lamports, atas, space: accountInfoResponse.value?.space }; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. space comes back as a bigint but everything else in this return is Number coerced, including lamports on the same line. Two real problems: JSON.stringify throws on bigint, and 0n is falsy, so any |
||
| } | ||
|
|
||
| /** | ||
|
|
@@ -998,7 +991,7 @@ export class SolRpc { | |
| async getTokenAccountsByOwner({ address, skipExistenceCheck = false, maxDepth = 0 }) { | ||
| // Only explicit skipExistenceCheck: true should bypass | ||
| if (skipExistenceCheck !== true) { | ||
| const accountInfoResponse = await this.rpc.getAccountInfo(address).send(); | ||
| const accountInfoResponse = await this.rpc.getAccountInfo(address, { encoding: 'base64' }).send(); | ||
| if (!accountInfoResponse.value) { | ||
| throw new Error(SOL_ERROR_MESSAGES.SOL_ACCT_NOT_FOUND); | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| import { expect } from 'chai'; | ||
|
|
||
| // Structural assertions for a client.getAccountInfo() result, shared by the real integration tests in | ||
| // sol.js and spl.js that hit a local validator or devnet. Keeping this in one place means both suites are | ||
| // checking the same result shape, so a change to getAccountInfo's return value only needs to be taught to | ||
| // one assertion, not two copy-pasted ones. This is a plain function with no describe/it of its own, so | ||
| // importing it has no side effects on either file's own test run. | ||
| export const assertAccountInfoShape = result => { | ||
| expect(result).to.be.an('object').that.is.not.null; | ||
| expect(result).to.have.all.keys('lamports', 'atas', 'space'); | ||
| expect(result).to.have.property('lamports').that.is.a('number').greaterThanOrEqual(0); | ||
| expect(result).to.have.property('atas').that.is.an('array'); | ||
| if (result.space !== undefined) { | ||
| expect(result).to.have.property('space').that.is.a('bigint'); | ||
| } | ||
| for (const ata of result.atas) { | ||
| expect(ata).to.be.an('object'); | ||
| expect(ata).to.have.property('mint').that.is.a('string'); | ||
| expect(ata).to.have.property('pubkey').that.is.a('string'); | ||
| expect(ata).to.have.property('state').that.is.a('string'); | ||
| expect(ata).to.have.property('atas').that.is.an('array'); | ||
| } | ||
| }; |
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.
bitpay calls getAccountInfo through ChainInterface in isAddressPayable (lib/utils.js:511) and relies on the catch at :549 to return false for ATA addresses. With the throw gone an ATA passes validation as a SOL destination, and SOL sent to an ATA is stuck unless the account gets closed. We need a guard on the bitpay side first, something like reject when space > 0, or keep some way to flag ATAs here.
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.
Nice catch. I'll make that change in the bitpay MR that pulls this crypto-rpc change in - I'll have you review that too so we'll make sure that gets through. That method is actually what precipitated this change in the first place.