Extract object loading from transaction input checking #14579
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
3 Ignored Deployments
|
mystenmark
changed the title
mystenmark
force-pushed
the
mlogan-transaction-input-loader
branch
from
c62e697 to
0011735
Compare
mystenmark
force-pushed
the
mlogan-transaction-input-loader
branch
from
ebb8f9a to
72d8b61
Compare
mystenmark
force-pushed
the
mlogan-transaction-input-loader
branch
from
72d8b61 to
1be8251
Compare
mystenmark
force-pushed
the
mlogan-transaction-input-loader
branch
from
1be8251 to
bbfbb82
Compare
mystenmark
force-pushed
the
mlogan-transaction-input-loader
branch
from
bbfbb82 to
ea87807
Compare
There was a problem hiding this comment.
Went through it again, this time a bit more deeply.
Overall everything looks pretty good to me, a couple smaller nits here and there that you should feel free to accept/ignore.
Only larger thing that I spotted was a change in gas_v2 where I believe an as_object could be None and we could call expect on it. Unless this is guarded by other checks further up in the transaction input checks possibly (but I couldn't find it).
mystenmark
force-pushed
the
mlogan-transaction-input-loader
branch
from
db337b1 to
3fe2e16
Compare
| input_object_kinds: &[InputObjectKind], | ||
| receiving_objects: &[ObjectRef], | ||
| protocol_config: &ProtocolConfig, | ||
| ) -> SuiResult<(InputObjects, ReceivingObjects)> { |
There was a problem hiding this comment.
This function should not return ReceivingObjects today because it's effectively always empty, until we actually implement that part. We should make it so and add a TODO
There was a problem hiding this comment.
I started trying to remove this, and found that for dry-run, all the other machinery is already in place, so instead I changed this to read the receiving objects and return them.
| &self, | ||
| _tx_digest: Option<&TransactionDigest>, | ||
| input_object_kinds: &[InputObjectKind], | ||
| receiving_objects: &[ObjectRef], |
There was a problem hiding this comment.
That's not a good argument to leave this not processed logically. We could move the fp_ensure check to the two callers for now until we decide to implement it
- Is primarily intended to support the creation of an in-memory object cache (coming soon). - Makes it easier to use multi-gets. - Simplifies handling of deleted shared objects and receivables. - Will prevent inadvertently loading the same object multiple times. - Will reduce the proliferation of special purpose traits (e.g. MarkerTableQuery which is removed in this PR).
Co-authored-by: Xun Li <lxfind@gmail.com>
mystenmark
force-pushed
the
mlogan-transaction-input-loader
branch
from
d55d613 to
217e8cd
Compare
This PR
cache (coming soon).
MarkerTableQuery which is removed in this PR).
Description
Describe the changes or additions included in this PR.
Test Plan
How did you test the new or updated feature?
If your changes are not user-facing and not a breaking change, you can skip the following section. Otherwise, please indicate what changed, and then add to the Release Notes section as highlighted during the release process.
Type of Change (Check all that apply)
Release notes