46 add contributors to the concert account #47
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Copilot
AI
left a comment
Copilot
AI
left a comment
There was a problem hiding this comment.
Copilot reviewed 3 out of 4 changed files in this pull request and generated 1 comment.
Files not reviewed (1)
- anchor/.gitignore: Language not supported
Comments suppressed due to low confidence (3)
anchor/tests/concert-x.ts:79
- The test should verify that the specific backer's public key is included in the 'contributors' list, not just that the list is non-empty.
expect(updatedConcertAccount.contributors.length).greaterThan(0);
anchor/programs/concert-x/src/lib.rs:32
- The field 'pda' should be named 'creator' to maintain consistency with the rest of the codebase and to accurately reflect its purpose.
concert.pda = ctx.accounts.initializer.key();
anchor/programs/concert-x/src/lib.rs:139
- The length of 'short_description' is reduced to 100 characters, which might not be sufficient for a brief description. Consider increasing the length to 200 characters as it was previously.
pub short_description: String,
|
|
||
| //Update the current amount | ||
| ctx.accounts.concert.current_amount += amount; | ||
| ctx.accounts.concert.contributors.push(*ctx.accounts.backer.key); |
There was a problem hiding this comment.
This line may cause a 'contributor' to be added multiple times if they contribute more than once. Consider checking if the contributor already exists in the list before adding.
| ctx.accounts.concert.contributors.push(*ctx.accounts.backer.key); | |
| if !ctx.accounts.concert.contributors.contains(&ctx.accounts.backer.key()) { ctx.accounts.concert.contributors.push(*ctx.accounts.backer.key()); } |
Copilot is powered by AI, so mistakes are possible. Review output carefully before use.
loganrudd
changed the base branch from
develop
to
28-implement-contribution-logic
| pub status: u8, | ||
| /// Concert contributors | ||
| #[max_len(100)] | ||
| pub contributors: Vec<Pubkey>, |
There was a problem hiding this comment.
Does it make sense to make this a vec of a map with {Pubkey: contribution amount} key:value pairs? Since the logic currently allows people to donate any amount above the ticket price. We can also change the logic easily so that they can only contribute the price of the ticket and nothing above it. What do you think?
| @@ -76,6 +76,7 @@ describe("concert-x", () => { | |||
|
|
|||
| const updatedConcertAccount = await program.account.concert.fetch(concertXPda); | |||
| expect(updatedConcertAccount.currentAmount.toNumber()).to.equal(contributionAmount); | |||
| expect(updatedConcertAccount.contributors.length).greaterThan(0); | |||
There was a problem hiding this comment.
Maybe we could check that contributors Pubkey is in the vec instead? Something like copilot suggests above:
ctx.accounts.concert.contributors.contains(&ctx.accounts.backer.key())
Logic to add controbutors to the concert