-
Notifications
You must be signed in to change notification settings - Fork 4.6k
Relax authority signer check for lookup table creation #27248
Conversation
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.
The program logic looks good, just a few questions on the client-side bits to facilitate deprecation
.arg( | ||
Arg::with_name("authority_signer") | ||
.long("authority-signer") |
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.
To facilitate the migration process away from signing these, do you want to have this flag hidden and print a message saying it's deprecated as of 1.12 if it's used? Or do you plan to do that once the feature is enabled on all clusters?
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's actually ok to use this option after the feature is activated. The bigger issue is if folks try using --authority
before the feature is activated so I added a warning to that option. We can add authority-signer
as an alias to authority
once the feature is activated everywhere
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.
Ok great, aliasing later makes sense to me
@@ -84,6 +84,7 @@ pub fn create_lookup_table( | |||
authority_address: Pubkey, | |||
payer_address: Pubkey, | |||
recent_slot: Slot, | |||
authority_is_signer: bool, |
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.
Do you want to remove this argument once the feature is enabled on all clusters? If so, it'll be easier for clients on the transition if there are two functions, one that declares the authority as a signer, and one that doesn't. You could have the existing create_lookup_table_signed
require the signature, mark it as deprecated, and also have create_lookup_table
which doesn't.
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.
Good call, added
7bf2a2d
to
5a5a1e6
Compare
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.
Looks great!
…7248) * Relax authority signer check for lookup table creation * cli: support creating lookup tables without authority signer * add another create lookup table ix function * improve help message
Has this functionality been pushed to Web3, from what I see the authority is still required using Web3 but I may be wrong Github - Solana Web3 |
If you have an issue with solana-web3.js, please open it there |
Problem
Since lookup tables are derived from a recent slot, it's difficult to time a transaction that creates an address lookup table account controlled by a multisig or governance program. This is due to the create lookup table instruction requiring the authority to be a signer.
Summary of Changes
Feature Gate Issue: #27205