add Address Lookup Table program and support for version 0 transactions#7
add Address Lookup Table program and support for version 0 transactions#7jochemarends wants to merge 4 commits intodcrck:masterfrom
Conversation
…ns (dcrck#6) Since transactions can now use address lookup tables, this commit adds support for the version 0 transaction format. This format is implicitly used when the `:lookup_tables` option is non-empty: - `:lookup_tables` (default: `%{}`) — A map of address lookup tables to use. Each key is a `t:Solana.Key.t/0` representing the address of a lookup table account, and each value is a list of `t:Solana.Key.t/0` representing the addresses of that lookup table. When non-empty, the transaction is encoded using the [version 0 format](https://docs.anza.xyz/proposals/versioned-transactions#versioned-transactions).
dcrck
left a comment
dcrck
left a comment
There was a problem hiding this comment.
Hello,
Thank you so much for the pull request, and I apologize for taking so long to get around to reviewing it. I've been in a new job and these past few months have been very busy.
I've left some detailed feedback and stylistic change requests. Now that I've taken the time to read over the code I should be able to respond to comments on this one quickly. Please let me know if you have any questions or anything is confusing, and I'll get back to you ASAP.
Thanks again!
| end | ||
| end | ||
|
|
||
| describe "extend_lookup_table/1" do |
There was a problem hiding this comment.
the web3.js suite includes a a test for extending a lookup table without a payer, and that's an optional parameter for the function, so we should test that case here as well.
| assert LookupTable.from_account_info(info).authority == nil | ||
| end | ||
| end | ||
|
|
There was a problem hiding this comment.
missing close_lookup_table/1 test case
| authority: pubkey!(new), | ||
| payer: pubkey!(payer), | ||
| new_keys: [SystemProgram.id()] | ||
| ), |
There was a problem hiding this comment.
Maybe extend the lookup table without a payer here to mimic the web3.js test suite?
|
Hi, Thank you for reviewing the pull request! I totally get that you're busy with other things, so please feel free to respond whenever it's convenient for you. I appreciate the detailed feedback and already incorporated some of it. I will update the pull request once I found time to incorporate the rest. |
- Change the default value for the `:bind_address` option of `Solana.TestValidator` to match the `--bind-address` option of `solana-test-validator` version 3.1.8. - Remove the `:gossip_host` option for `Solana.TestValidator` as it's not supported in version 3.1.8 of `solana-test-validator`. `--bind-address` now controls the Gossip port. - Add `:authority_should_sign?` option for `LookupTable.create_lookup_table/1` - Add test for `LookupTable.close_lookup_table/1`. - In the test for `LookupTable.deactivate_lookup_table/1` extend the lookup table without a payer to mimic the web3.js test suite.
|
I've incorporated the feedback and ran the tests, which all passed. Since a certain amount of slots must elapse between the deactivation and the closing of a lookup table, the tests take about 3-4 minutes to execute. Before merging, I would like to add test cases for Here's a screenshots of the tests passing: 
Also, I've had to make some changes to Here's what |
| # must be stored in the message itself | ||
| {static_accounts, lookup_accounts} = | ||
| accounts | ||
| |> Enum.split_with(&(&1.key not in keys or &1.signer? or &1 in invoked_accounts)) |
There was a problem hiding this comment.
Unless I'm missing some core library update,, in doesn't work for MapSet. You need to use MapSet.member?(invoked_accounts, &1) here.
There was a problem hiding this comment.
I've tested it and it does work, but I can change it to MapSet.member? if that is preferred.
iex(1)> :foo in MapSet.new([:foo, :bar])
true
2 participants
Hello,
Since transactions can now use address lookup tables, this commit adds support for the version 0 transaction format. This format is implicitly used when the
:lookup_tablesoption is non-empty.:lookup_tables(default:%{}) — A map of address lookup tables to use. Each key is at:Solana.Key.t/0representing the address of a lookup table account, and each value is a list oft:Solana.Key.t/0representing the addresses of that lookup table.When non-empty, the transaction is encoded using the version 0 format.
Solana.Transaction.to_binary/2now accepts two options; however, there's currently no way to use these withSolana.RPC.Request.send_transaction/2.It might be nice to allow
Solana.RPC.Request.send_transaction/2to either accept pre-compiled transactions or forward these options toSolana.Transaction.to_binary/2.Version 0 transaction cannot be parsed without an RPC connection.
Because
Solana.Transaction.parse/1currently does not have access to an RPC client, I have left it out the parsing of version 0 transactions for now.I also left out the test for
Solana.LookupTable.close_lookup_table/1because closing a lookup table requires a cool-down period after deactivation, which caused the tests to time out.Once there is way to send transactions that are compiled with options, I will add tests for version 0 transactions.