Solana ChainWriter #935
Open
Solana ChainWriter #935
+4,410
−1,378
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
silaslenihan
had a problem deploying
to
integration
GitHub Actions
Failure
silaslenihan
had a problem deploying
to
integration
GitHub Actions
Error
silaslenihan
temporarily deployed
to
integration
GitHub Actions
Inactive
silaslenihan
force-pushed
the
BCFR-995-Solana-ChainWriter
branch
from
8c8a858 to
845a9a6
Compare
silaslenihan
had a problem deploying
to
integration
GitHub Actions
Error
silaslenihan
had a problem deploying
to
integration
GitHub Actions
Error
silaslenihan
had a problem deploying
to
integration
GitHub Actions
Error
silaslenihan
force-pushed
the
BCFR-995-Solana-ChainWriter
branch
from
845a9a6 to
13cf316
Compare
silaslenihan
temporarily deployed
to
integration
GitHub Actions
Inactive
silaslenihan
temporarily deployed
to
integration
GitHub Actions
Inactive
silaslenihan
temporarily deployed
to
integration
GitHub Actions
Inactive
silaslenihan
had a problem deploying
to
integration
GitHub Actions
Error
silaslenihan
temporarily deployed
to
integration
GitHub Actions
Inactive
silaslenihan
temporarily deployed
to
integration
GitHub Actions
Inactive
silaslenihan
temporarily deployed
to
integration
GitHub Actions
Inactive
silaslenihan
temporarily deployed
to
integration
GitHub Actions
Inactive
silaslenihan
had a problem deploying
to
integration
GitHub Actions
Failure
silaslenihan
temporarily deployed
to
integration
GitHub Actions
Inactive
silaslenihan
temporarily deployed
to
integration
GitHub Actions
Inactive
silaslenihan
temporarily deployed
to
integration
GitHub Actions
Inactive
silaslenihan
temporarily deployed
to
integration
GitHub Actions
Inactive
silaslenihan
temporarily deployed
to
integration
GitHub Actions
Inactive
silaslenihan
force-pushed
the
BCFR-995-Solana-ChainWriter
branch
from
7332617 to
17379b5
Compare
silaslenihan
temporarily deployed
to
integration
GitHub Actions
Inactive
silaslenihan
temporarily deployed
to
integration
GitHub Actions
Inactive
silaslenihan
temporarily deployed
to
integration
GitHub Actions
Inactive
silaslenihan
had a problem deploying
to
integration
GitHub Actions
Error
silaslenihan
force-pushed
the
BCFR-995-Solana-ChainWriter
branch
from
17379b5 to
93cba43
Compare
silaslenihan
temporarily deployed
to
integration
GitHub Actions
Inactive
silaslenihan
temporarily deployed
to
integration
GitHub Actions
Inactive
* Added ChainWriter unit tests for GetFeeComponents and GetTransactionStatus * Created SubmitTransaction tests * Created SubmitTransaction tests * Moved txm utils into own package and generated txm mock * Updated chain writer tests to use txm mock * Added GetAddresses unit test and fixed SubmitTransaction unit test * Fixed linting and removed file read for IDL * Fixed filter lookup table error case and fixed linting * Added filter lookup table addresses unit tests * Added new test case and fixed formatting issues * Addressed golang lint suggestions * Cleaned out unused dependency and fixed remaining golang lint errors * Added derived lookup table indeces unit tests --------- Co-authored-by: Silas Lenihan <sjl@lenihan.net>
silaslenihan
force-pushed
the
BCFR-995-Solana-ChainWriter
branch
from
b852c76 to
51de345
Compare
silaslenihan
temporarily deployed
to
integration
GitHub Actions
Inactive
silaslenihan
temporarily deployed
to
integration
GitHub Actions
Inactive
silaslenihan
temporarily deployed
to
integration
GitHub Actions
Inactive
silaslenihan
temporarily deployed
to
integration
GitHub Actions
Inactive
silaslenihan
temporarily deployed
to
integration
GitHub Actions
Inactive
silaslenihan
temporarily deployed
to
integration
GitHub Actions
Inactive
silaslenihan
temporarily deployed
to
integration
GitHub Actions
Inactive
|
ilija42
reviewed
Dec 25, 2024
| } | ||
|
|
||
| // Enqueue transaction | ||
| if err = s.txm.Enqueue(ctx, accounts[0].PublicKey.String(), tx, &transactionID, blockhash.Value.LastValidBlockHeight); err != nil { |
There was a problem hiding this comment.
Config doesn't have any validation for accounts lookup having any entries, so wouldn't this panic if one of the methodConfigs has no account lookups listed? You should check for accounts size
ilija42
reviewed
Dec 25, 2024
| return filteredLookupTables | ||
| } | ||
|
|
||
| func (s *SolanaChainWriterService) SubmitTransaction(ctx context.Context, contractName, method string, args any, transactionID string, toAddress string, meta *types.TxMeta, value *big.Int) error { |
There was a problem hiding this comment.
lint nit
Suggested change
| func (s *SolanaChainWriterService) SubmitTransaction(ctx context.Context, contractName, method string, args any, transactionID string, toAddress string, meta *types.TxMeta, value *big.Int) error { | |
| func (s *SolanaChainWriterService) SubmitTransaction(ctx context.Context, contractName, method string, args any, transactionID string, toAddress string, _ *types.TxMeta, _ *big.Int) error { |
ilija42
reviewed
Dec 25, 2024
|
|
||
| for _, seed := range lookup.Seeds { | ||
| if lookupSeed, ok := seed.(AccountLookup); ok { | ||
| // Get value from a location (This doens't have to be an address, it can be any value) |
There was a problem hiding this comment.
lint
Suggested change
| // Get value from a location (This doens't have to be an address, it can be any value) | |
| // Get value from a location (This doens't have to be an address, it can be any value) |
ilija42
reviewed
Dec 25, 2024
| } | ||
|
|
||
| // GetValuesAtLocation parses through nested types and arrays to find all locations of values | ||
| func GetValuesAtLocation(args any, location string) ([][]byte, error) { |
There was a problem hiding this comment.
Callers in multiple locations are trying to access response slice elem by index without checking the size which is panic prone
ilija42
reviewed
Dec 25, 2024
| var lookupTableMetas []*solana.AccountMeta | ||
|
|
||
| // Iterate over each address of the lookup table | ||
| for _, addressMeta := range lookupTableAddresses { |
There was a problem hiding this comment.
Is it possible to get empty slice of lookupTableAddresses, if it is is that okay? If not maybe throw an error here
ilija42
reviewed
Dec 25, 2024
| // Read the full list of addresses from the lookup table | ||
| addresses, err := getLookupTableAddresses(ctx, reader, addressMeta.PublicKey) | ||
| if err != nil { | ||
| return nil, nil, fmt.Errorf("error fetching lookup table address: %w", err) |
There was a problem hiding this comment.
does it make sense to also log the table address here?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Related Tickets:
NONEVM-753
NONEVM-963
NONEVM-964
Requires Dependencies
Resolves Dependencies