Support interest bearing mint in token-js #3266
Conversation
joncinque
left a comment
joncinque
left a comment
There was a problem hiding this comment.
This is a great start! Just some small points, and if you add a test for updating the rate, this should be good to go
| export const interestBearingMintInitializeInstructionData = struct<InterestBearingMintInitializeInstructionData>([ | ||
| u8('instruction'), | ||
| u8('interestBearingMintInstruction'), | ||
| publicKey('rateAuthority'), |
There was a problem hiding this comment.
I'm just realizing that this isn't done correctly with the transfer fee extension, since it's not actually a publicKey, but rather an OptionalNonZeroPublicKey, meaning that it's really publicKey?. We can keep this for now, but eventually we need to properly handle the optionality
There was a problem hiding this comment.
gotcha. is that type typicall added to @solana/buffer-layout-utils or are they usually defined in the token js library?
There was a problem hiding this comment.
In this case, we can probably add it to the token js library. If it eventually gets ported to the solana sdk, then we can also add it to buffer-layout-utils
| /** | ||
| * Construct an UpdateRateInterestBearingMint instruction | ||
| * | ||
| * @param mint Mint to initialize | ||
| * @param rate The updated interest rate | ||
| * @param programId SPL Token program account | ||
| * | ||
| * @return Instruction to add to a transaction | ||
| */ |
There was a problem hiding this comment.
This also requires the rate authority to sign -- see the implementation at
|
|
||
| export interface InterestBearingMintConfigState { | ||
| rateAuthority: PublicKey; | ||
| initializationTimestamp: number; |
There was a problem hiding this comment.
Since timestamps are u64 under the hood, let's use a BigInt
| rateAuthority: PublicKey; | ||
| initializationTimestamp: number; | ||
| preUpdateAverageRate: number; | ||
| lastUpdateTimestamp: number; |
| lamports, | ||
| programId: TEST_PROGRAM_ID, | ||
| }), | ||
| createInitializeInterestBearingMintInstruction(mint, rateAuthority.publicKey, 10, TEST_PROGRAM_ID), |
There was a problem hiding this comment.
nitty nit: we can make the 10 a constant for the tests
| const interestBearingMintConfigState = getInterestBearingMintConfigState(mintInfo); | ||
| expect(interestBearingMintConfigState).to.not.be.null; | ||
| if (interestBearingMintConfigState !== null) { | ||
| expect(interestBearingMintConfigState.currentRate).to.eql(10); |
There was a problem hiding this comment.
Can you check all of the fields to be sure that it's properly initialized?
kunaljaydesai
force-pushed
the
kunal/interest-bearing-mint
branch
from
6273caa to
840a69f
Compare
|
@joncinque I have updated the PR with the feedback mentioned, lmk if things look ok! appreciate the review :) |
joncinque
left a comment
joncinque
left a comment
There was a problem hiding this comment.
Just a couple of tiny things, then we can merge this!
| const keys = addSigners( | ||
| [ | ||
| { pubkey: mint, isSigner: false, isWritable: true }, | ||
| { pubkey: rateAuthority, isSigner: true, isWritable: false }, |
There was a problem hiding this comment.
For the multisig situation, the multisig doesn't sign, but all the signers do
| { pubkey: rateAuthority, isSigner: true, isWritable: false }, | |
| { pubkey: rateAuthority, isSigner: !multiSigners.length, isWritable: false }, |
| expect(interestBearingMintConfigState).to.not.be.null; | ||
| if (interestBearingMintConfigState !== null) { | ||
| expect(interestBearingMintConfigState.rateAuthority).to.eql(rateAuthority.publicKey); | ||
| expect(interestBearingMintConfigState.preUpdateAverageRate).to.eql(10); |
There was a problem hiding this comment.
nitty nit:
| expect(interestBearingMintConfigState.preUpdateAverageRate).to.eql(10); | |
| expect(interestBearingMintConfigState.preUpdateAverageRate).to.eql(TEST_RATE); |
|
sweet, done! sorry for the delay |
|
Thanks for your contribution! Looks good |
2 participants
@joncinque WIP to resolve #3263
Lmk what you think. I'm also having trouble running e2e tests, so this hasn't been validated. I'm getting an error with:
Is there some set up I have to do?