Skip to content

Commit

Permalink
[EffectsV2] effects gas_object() return value instead of ref (#13015)
Browse files Browse the repository at this point in the history
## Description 

In effects v2 we may use index offset to reference gas object, and we
also store lamport sequence number separately.
Hence we won't be able to return a reference to gas object. Return value
instead.
This is a non-functional change.

## Test Plan 

CI

---
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)

- [ ] protocol change
- [ ] user-visible impact
- [ ] breaking change for a client SDKs
- [ ] breaking change for FNs (FN binary must upgrade)
- [ ] breaking change for validators or node operators (must upgrade
binaries)
- [ ] breaking change for on-chain data layout
- [ ] necessitate either a data wipe or data migration

### Release notes
  • Loading branch information
lxfind authored Jul 17, 2023
1 parent 0060cc2 commit 37dc967
Show file tree
Hide file tree
Showing 4 changed files with 5 additions and 5 deletions.
2 changes: 1 addition & 1 deletion crates/sui-benchmark/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ impl ExecutionEffects {
pub fn gas_object(&self) -> (ObjectRef, Owner) {
match self {
ExecutionEffects::CertifiedTransactionEffects(certified_effects, ..) => {
*certified_effects.data().gas_object()
certified_effects.data().gas_object()
}
ExecutionEffects::SuiTransactionBlockEffects(sui_tx_effects) => {
let refe = &sui_tx_effects.gas_object();
Expand Down
2 changes: 1 addition & 1 deletion crates/sui-json-rpc/src/balance_changes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ pub async fn get_balance_changes_from_effect<P: ObjectProvider<Error = E>, E>(
// Only charge gas when tx fails, skip all object parsing
if effects.status() != &ExecutionStatus::Success {
return Ok(vec![BalanceChange {
owner: *gas_owner,
owner: gas_owner,
coin_type: GAS::type_tag(),
amount: effects.gas_cost_summary().net_gas_usage().neg() as i128,
}]);
Expand Down
4 changes: 2 additions & 2 deletions crates/sui-types/src/effects/effects_v1.rs
Original file line number Diff line number Diff line change
Expand Up @@ -102,8 +102,8 @@ impl TransactionEffectsAPI for TransactionEffectsV1 {
fn wrapped(&self) -> &[ObjectRef] {
&self.wrapped
}
fn gas_object(&self) -> &(ObjectRef, Owner) {
&self.gas_object
fn gas_object(&self) -> (ObjectRef, Owner) {
self.gas_object
}
fn events_digest(&self) -> Option<&TransactionEventsDigest> {
self.events_digest.as_ref()
Expand Down
2 changes: 1 addition & 1 deletion crates/sui-types/src/effects/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -213,7 +213,7 @@ pub trait TransactionEffectsAPI {
fn deleted(&self) -> &[ObjectRef];
fn unwrapped_then_deleted(&self) -> &[ObjectRef];
fn wrapped(&self) -> &[ObjectRef];
fn gas_object(&self) -> &(ObjectRef, Owner);
fn gas_object(&self) -> (ObjectRef, Owner);
fn events_digest(&self) -> Option<&TransactionEventsDigest>;
fn dependencies(&self) -> &[TransactionDigest];
// All changed objects include created, mutated and unwrapped objects,
Expand Down

1 comment on commit 37dc967

@github-actions
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

4 Validators 500/s Owned Transactions Benchmark Results

Benchmark Report:
+-------------+-----+-----+--------+---------------+---------------+---------------+-----------------------+----------------------------+
| duration(s) | tps | cps | error% | latency (min) | latency (p50) | latency (p99) | gas used (MIST total) | gas used/hr (MIST approx.) |
+=======================================================================================================================================+
| 60          | 591 | 591 | 0      | 34            | 8351          | 8695          | 442,310,976,000       | 26,538,658,560,000         |
Stress Performance Report:
+-----------+-----+-----+
| metric    | p50 | p99 |
+=======================+
| cpu usage | 13  | 100 |

4 Validators 500/s Shared Transactions Benchmark Results

Benchmark Report:
+-------------+-----+-----+--------+---------------+---------------+---------------+-----------------------+----------------------------+
| duration(s) | tps | cps | error% | latency (min) | latency (p50) | latency (p99) | gas used (MIST total) | gas used/hr (MIST approx.) |
+=======================================================================================================================================+
| 60          | 471 | 471 | 0      | 20            | 9599          | 13663         | 425,135,887,200       | 25,508,153,232,000         |
Stress Performance Report:
+-----------+-----+-----+
| metric    | p50 | p99 |
+=======================+
| cpu usage | 12  | 100 |

20 Validators 50/s Owned Transactions Benchmark Results

Benchmark Report:
+-------------+-----+-----+--------+---------------+---------------+---------------+-----------------------+----------------------------+
| duration(s) | tps | cps | error% | latency (min) | latency (p50) | latency (p99) | gas used (MIST total) | gas used/hr (MIST approx.) |
+=======================================================================================================================================+
| 60          | 200 | 200 | 0      | 21            | 64            | 104           | 154,002,048,000       | 9,240,122,880,000          |
Stress Performance Report:
+-----------+-----+-----+
| metric    | p50 | p99 |
+=======================+
| cpu usage | 26  | 52  |

20 Validators 50/s Shared Transactions Benchmark Results

Benchmark Report:
+-------------+-----+-----+--------+---------------+---------------+---------------+-----------------------+----------------------------+
| duration(s) | tps | cps | error% | latency (min) | latency (p50) | latency (p99) | gas used (MIST total) | gas used/hr (MIST approx.) |
+=======================================================================================================================================+
| 60          | 194 | 194 | 0      | 66            | 1423          | 2105          | 172,410,168,000       | 10,344,610,080,000         |
Stress Performance Report:
+-----------+-----+-----+
| metric    | p50 | p99 |
+=======================+
| cpu usage | 25  | 58  |

Narwhal Benchmark Results

 SUMMARY:
-----------------------------------------
 + CONFIG:
 Faults: 0 node(s)
 Committee size: 4 node(s)
 Worker(s) per node: 1 worker(s)
 Collocate primary and workers: True
 Input rate: 50,000 tx/s
 Transaction size: 512 B
 Execution time: 0 s

 Header number of batches threshold: 32 digests
 Header maximum number of batches: 1,000 digests
 Max header delay: 2,000 ms
 GC depth: 50 round(s)
 Sync retry delay: 10,000 ms
 Sync retry nodes: 3 node(s)
 batch size: 500,000 B
 Max batch delay: 200 ms
 Max concurrent requests: 500,000 

 + RESULTS:
 Batch creation avg latency: 201 ms
 Header creation avg latency: -1 ms
 	Batch to header avg latency: -1 ms
 Header to certificate avg latency: 1 ms
 	Request vote outbound avg latency: 0 ms
 Certificate commit avg latency: 728 ms

 Consensus TPS: 0 tx/s
 Consensus BPS: 0 B/s
 Consensus latency: 0 ms

 End-to-end TPS: 0 tx/s
 End-to-end BPS: 0 B/s
 End-to-end latency: 0 ms
-----------------------------------------

Please sign in to comment.