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

Solana ChainWriter #935

Open
wants to merge 26 commits into
base: develop
Choose a base branch
from
Open

Solana ChainWriter #935

wants to merge 26 commits into from

Conversation

silaslenihan
Copy link
Contributor

@silaslenihan silaslenihan commented Nov 20, 2024 •
edited by jira bot
Loading

Description

Related Tickets:
NONEVM-753
NONEVM-963
NONEVM-964

Requires Dependencies

Resolves Dependencies

@silaslenihan silaslenihan force-pushed the BCFR-995-Solana-ChainWriter branch from 8c8a858 to 845a9a6 Compare November 20, 2024 15:31
@silaslenihan silaslenihan force-pushed the BCFR-995-Solana-ChainWriter branch from 845a9a6 to 13cf316 Compare November 20, 2024 15:32
@silaslenihan silaslenihan force-pushed the BCFR-995-Solana-ChainWriter branch from 7332617 to 17379b5 Compare November 21, 2024 20:34
@silaslenihan silaslenihan force-pushed the BCFR-995-Solana-ChainWriter branch from 17379b5 to 93cba43 Compare November 21, 2024 20:39
@silaslenihan silaslenihan force-pushed the BCFR-995-Solana-ChainWriter branch from 99d006d to 5c85cfa Compare December 13, 2024 23:13
@silaslenihan silaslenihan force-pushed the BCFR-995-Solana-ChainWriter branch from 5c85cfa to f6ae6d9 Compare December 16, 2024 13:35
Farber98
Farber98 reviewed Dec 16, 2024
View reviewed changes
Copy link
Contributor

@Farber98 Farber98 left a comment

Choose a reason for hiding this comment

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

Excellent job Silas! The structure looks really good and the complexity is well hidden behind the interface 🙌 . I feel I'm missing some context to review some details, will ask you some questions on Slack

contracts/artifacts/localnet/write_test-keypair.json Outdated Show resolved Hide resolved
contracts/programs/contract-reader-interface/src/lib.rs
#[account(
init,
payer = admin,
space = size_of::<LookupTableDataAccount>() + 8,
Copy link
Contributor

Choose a reason for hiding this comment

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

just curious, what is +8 here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That accounts for the 8-byte discriminator I believe

pkg/solana/chainwriter/ccip_example_config.go
},
},
}
fmt.Println(chainWriterConfig)
Copy link
Contributor

Choose a reason for hiding this comment

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

keeping this print?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just did this so the compiler wouldn't complain about the unused variable

pkg/solana/chainwriter/lookups.go Show resolved Hide resolved
@silaslenihan silaslenihan force-pushed the BCFR-995-Solana-ChainWriter branch from f6ae6d9 to 1d34387 Compare December 16, 2024 19:52
@cl-sonarqube-production
Copy link

Quality Gate failed Quality Gate failed

Failed conditions
31.8% Coverage on New Code (required ≥ 75%)

See analysis details on SonarQube

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@amit-momin amit-momin amit-momin left review comments

@Farber98 Farber98 Farber98 left review comments

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@silaslenihan @Farber98 @amit-momin @pablolagreca