-
Notifications
You must be signed in to change notification settings - Fork 11.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
Move TxContext
implementation via natives and sponsor
#21245
base: main
Are you sure you want to change the base?
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
2 Skipped Deployments
|
bd777b4
to
18f21c9
Compare
df2b8dd
to
b04724e
Compare
b04724e
to
9953050
Compare
f7a8d94
to
6e915c8
Compare
6e915c8
to
2cc1767
Compare
2cc1767
to
a9ceb02
Compare
a9ceb02
to
0c0dd72
Compare
0c0dd72
to
fa43f35
Compare
@@ -3254,7 +3283,19 @@ impl ProtocolConfig { | |||
cfg.feature_flags.passkey_auth = true; | |||
} | |||
} | |||
76 => {} | |||
76 => { | |||
cfg.feature_flags.move_native_context = false; |
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.
Why value 30?
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.
see answer to @tnowacki 's same comment
@@ -338,11 +355,77 @@ impl NativesCostTable { | |||
.transfer_share_object_cost_base() | |||
.into(), | |||
}, | |||
// tx_context |
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 about the meaning of this comment (is it cut short?)
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.
I can remove it but it was to highlight that the API below is for the tx_context
and other part of this code do the same, so it was to replicate what is there already and to mark where that API is
crates/sui-types/src/base_types.rs
Outdated
pub fn fresh_id(&mut self) -> ObjectID { | ||
let id = ObjectID::derive_id(self.digest(), self.ids_created); | ||
pub fn sender(&self) -> SuiAddress { | ||
SuiAddress::from(ObjectID(self.sender)) |
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.
I would recommend self.sender.into()
or possibly SuiAddress:from(self.sender)
here. Going through ObjectID
to get there feels just wasteful lol
76 => { | ||
cfg.feature_flags.move_native_context = false; | ||
|
||
cfg.tx_context_fresh_id_cost_base = Some(30); |
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 should be the same as tx_context_derive_id_cost_base
. You could even reuse that one instead of this one possibly if you wanted to?
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.
fair, I will put that value in here for now, good point.
Later we will tune if we need to.
I did not want to reuse that value because I am not sure how those functions will evolve.
The derived_id may disappear, not sure
* native fun fresh_id | ||
* Implementation of the Move native function `fun fresh_id(): address` | ||
**************************************************************************************************/ | ||
pub fn fresh_id( |
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.
Wait, what even is this function? Is it fresh_object_address
? Why isn't that kept in Move?
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.
see comment about that above (as an asnwer to @tzakian) we need to compute the fresh id in the new model and that is all in rust.
How could we do that in Move code?
bde973a
to
d1212cd
Compare
TxContext
changes and Move native functions implementationTxContext
implementation via natives and sponsor
d1212cd
to
1450da3
Compare
1450da3
to
c64140e
Compare
c64140e
to
100114f
Compare
100114f
to
f77bf7a
Compare
f77bf7a
to
cd14ea6
Compare
cd14ea6
to
5bb2321
Compare
5bb2321
to
c14b731
Compare
Description
This enables Move
TxContext
to use native functions. It also expose afun sponsor(self: &TxContext): Option<address>
that returns the address of the sponsor if the transaction was sponsored.tx_context.rs
has all the changes to allowgas_price
,gas_budget
andsponsor
.I moved
TransactionContext
(the struct pushed toNativeContextExtension
) in its own directory likecrypto
andobject_runtime
. In timeTransactionContext
may evolve to have a richer API so it made sense to have it in its own directory.Test plan
Sponsor test coming
Release notes
Check each box that your changes affect. If none of the boxes relate to your changes, release notes aren't required.
For each box you select, include information after the relevant heading that describes the impact of your changes that a user might notice and any actions they must take to implement updates.