-
Notifications
You must be signed in to change notification settings - Fork 4.4k
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.
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
Add address lookup table program #21616
Add address lookup table program #21616
Conversation
98fa440
to
0b444db
Compare
0b444db
to
34d4ebb
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.
As with the first round of this program, looks really good! Only one potentially substantive comment, then this is definitely ready to go
{ | ||
let mut lookup_table_account_ref_mut = lookup_table_account.try_account_ref_mut()?; | ||
AddressLookupTable::overwrite_meta_data( | ||
lookup_table_account_ref_mut.data_as_mut_slice(), | ||
lookup_table_meta, | ||
)?; | ||
|
||
let table_data = lookup_table_account_ref_mut.data_mut(); | ||
for new_address in new_addresses { | ||
table_data.extend_from_slice(new_address.as_ref()); | ||
} | ||
} |
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.
This block changed in the latest commit. Rather than allocating a new account data vector, this tries to modify the AccountSharedData
data
vector directly if it isn't already referenced somewhere.
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.
Awesome, looks good!
Codecov Report
@@ Coverage Diff @@
## master #21616 +/- ##
=========================================
- Coverage 81.6% 81.4% -0.2%
=========================================
Files 511 515 +4
Lines 143320 143691 +371
=========================================
+ Hits 116976 117099 +123
- Misses 26344 26592 +248 |
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.
Not sure if you want anyone else to review, but it's good from my side!
{ | ||
let mut lookup_table_account_ref_mut = lookup_table_account.try_account_ref_mut()?; | ||
AddressLookupTable::overwrite_meta_data( | ||
lookup_table_account_ref_mut.data_as_mut_slice(), | ||
lookup_table_meta, | ||
)?; | ||
|
||
let table_data = lookup_table_account_ref_mut.data_mut(); | ||
for new_address in new_addresses { | ||
table_data.extend_from_slice(new_address.as_ref()); | ||
} | ||
} |
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.
Awesome, looks good!
* Add address lookup table program * feedback (cherry picked from commit 9b41ddd) # Conflicts: # runtime/Cargo.toml
Problem
Users and protocols need a way to store addresses on-chain for transaction v2 address lookups.
Summary of Changes
256
since transaction v2 account indices are typed asu8
Implements the amended proposal here: #21576