Pubkey duplication in Transaction

[garious]

Transaction.keys contains information that is also serialized into Transaction.userdata. Instead, change Transaction.keys from Vec<Pubkey> to Vec<u16>, where u16 is a byte offset to the first instance of that Pubkey in userdata.

Also, consider making that Vec<(u16, Option<u16>)> to optionally pair that Pubkey with a Signature.

[garious]

@aeyakovenko, @mvines, sound about right?

[garious]

@sakridge, would SigVerify be able to work with this additional level of indirection?

[sakridge]

@garious with modification, yes.

[aeyakovenko]

Why though?

On Thu, Sep 13, 2018 at 10:34 AM sakridge ***@***.***> wrote: @garious <https://github.com/garious> with modification, yes. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1210 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AA-ztjtTXIYDUCV1ZP1-2AvkE95B1rC5ks5uapcbgaJpZM4Wn0Ir> .

-- —— https://solana.com https://t.me/tolyloom

[mvines]

This seems a little weird. What does changing Transaction.keys in this way buy us beyond complexity?

[garious]

Much less bits => higher TPS, @aeyakovenko

[mvines]

hmm, currently the userdata doesn't need to index keys[] at all (the pubkey inlined in the Budget userdata can disappear). So this proposal sounds like we're actually increasing the serialized Transaction size.

[aeyakovenko]

Only if the language can’t use references to the array. The problem I see is that we will need to copy these keys out of the user data array just to do a lookup. You can’t just safely cast the bytes.

On Thu, Sep 13, 2018 at 6:17 PM Greg Fitzgerald ***@***.***> wrote: Much less bits => higher TPS, @aeyakovenko <https://github.com/aeyakovenko> — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1210 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AA-ztoWx3gL1v_PKR__0hFmnkn25rvDnks5uawOpgaJpZM4Wn0Ir> .

-- —— https://solana.com https://t.me/tolyloom

[aeyakovenko]

What I am not a fan of with this approach is that a user writing this by hand will need to recompute the offsets every time anything changes in the userdata. On Thu, Sep 13, 2018 at 6:23 PM Anatoly Yakovenko <anatoly@solana.com> wrote:

Only if the language can’t use references to the array. The problem I see is that we will need to copy these keys out of the user data array just to do a lookup. You can’t just safely cast the bytes. On Thu, Sep 13, 2018 at 6:17 PM Greg Fitzgerald ***@***.***> wrote: > Much less bits => higher TPS, @aeyakovenko > <https://github.com/aeyakovenko> > > — > You are receiving this because you were mentioned. > Reply to this email directly, view it on GitHub > <#1210 (comment)>, > or mute the thread > <https://github.com/notifications/unsubscribe-auth/AA-ztoWx3gL1v_PKR__0hFmnkn25rvDnks5uawOpgaJpZM4Wn0Ir> > . > -- —— https://solana.com https://t.me/tolyloom

-- —— https://solana.com https://t.me/tolyloom

[garious]

One minor correction, instead of Vec<(u16, Option<u16>)>, I'd suggest making that two separate Transaction fields:

sig_pubkey_offsets: Vec<(u16, u16)>,
pubkey_offsets: Vec<u16>,

And if you choose to inline it (because the duplication is less offensive than misaligned byte offsets), then each item in each vector would be nicely 8-byte aligned.

[mvines]

The userdata isn't the actual contract though right? It's the input that the contract consumes to move it to the next state, along with the list of keys of accounts the contract should use.

[garious]

@mvines, https://github.com/solana-labs/solana/blob/master/src/transaction.rs#L262

[mvines]

Yeah that's "the input that the contract consumes to move it to the next state". It's not the contract itself, the word "instruction" there is a misnomer.

[mvines]

Calling it "/dev/stdin" would be better ;)