-
Notifications
You must be signed in to change notification settings - Fork 305
fix and improve duplicate-mutable-accounts lesson #531
Conversation
….InitSpace.html) to calculate space needed for accounts. Delete Unnecessary parameters found in the test typescript. Consistently use "rpc()" as sending transactions in the test typescript.
….InitSpace.html) to calculate space needed for accounts. Replace "BorshDeserialize" and "BorshSerialize" with "AnchorDeserialize" and "AnchorSerialize". Make test descriptions more clear.
... | ||
it("Invoke insecure instruction with the same player should be successful", async () => { |
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.
... | |
it("Invoke insecure instruction with the same player should be successful", async () => { | |
... | |
it("Invokes insecure instruction with the same player successfully", async () => { |
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.
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.
Thanks for this! We can get this in easily, just needs modifications to meet the contributing guidelines but they're all small fixes.
... | ||
it("Invoke insecure instruction with the same player should be successful", async () => { |
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.
const p1 = await program.account.playerState.fetch(playerOne.publicKey); | ||
const p2 = await program.account.playerState.fetch(playerTwo.publicKey); |
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.
player
and player
per 'use full names' https://github.com/solana-foundation/developer-content/blob/main/CONTRIBUTING.md#code
const p1 = await program.account.playerState.fetch(playerOne.publicKey); | ||
const p2 = await program.account.playerState.fetch(playerTwo.publicKey); | ||
assert.equal(JSON.stringify(p1.choice), JSON.stringify({ rock: {} })); | ||
assert.equal(JSON.stringify(p2.choice), JSON.stringify({ scissors: {} })); |
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.
Can we do this without the JSON.stringify()
? assert.deepEqual()
or similar?
}) | ||
.rpc() | ||
.rpc(); | ||
} catch (err) { |
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.
See https://github.com/solana-foundation/developer-content/blob/main/CONTRIBUTING.md#jsts
catch (thrownObject) {
const error = thrownObject as Error
...
}
Push your code to GitHub and | ||
[tell us what you thought of this lesson](https://form.typeform.com/to/IPH0UGz7#answers-lesson=9b759e39-7a06-4694-ab6d-e3e7ac266ea7)! | ||
</Callout> | ||
--- |
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.
This appears to be a second copy of the lesson!
This pull request has been automatically marked as stale because it has not had recent activity. Remove stale label or comment or this will be closed in 7 days. |
Problem
Magic number found on space calculation.
"BorshDeserialize" and "BorshSerialize" are outdated.
Test descriptions are not clear enough.
Summary of Changes
Use InitSpace to calculate space needed for accounts.
Replace "BorshDeserialize" and "BorshSerialize" with "AnchorDeserialize" and "AnchorSerialize".
Make test descriptions more clear.
Also, I made a PR for solana-duplicate-mutable-accounts starter branch and a PR for solana-duplicate-mutable-accounts solution branch
which must be synced with this PR.