Skip to content
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.

Sign up for GitHub

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

Jump to bottom

add gas fee payer by adding a new payload type #8904

Merged
merged 31 commits into from
Jul 4, 2023
Merged

add gas fee payer by adding a new payload type #8904

merged 31 commits into from
Jul 4, 2023

Conversation

gerben-stavenga
Copy link
Contributor

@gerben-stavenga gerben-stavenga commented Jun 29, 2023 •
edited
Loading

Description

Add gas payer by adding a entry to RawMessageWithData however this struct is not being transmitted. So we need to add a new entry in the payload struct to transmit the intend of gas payer.

Test Plan

@gerben-stavenga gerben-stavenga marked this pull request as ready for review June 30, 2023 22:11
@gerben-stavenga gerben-stavenga changed the title first update add gas fee payer by adding a new payload type Jun 30, 2023
davidiw
davidiw reviewed Jul 1, 2023
View reviewed changes
aptos-move/aptos-vm/src/aptos_vm_impl.rs Outdated Show resolved Hide resolved
aptos-move/aptos-vm/src/transaction_metadata.rs Outdated Show resolved Hide resolved
types/src/transaction/mod.rs Show resolved Hide resolved
gerben-stavenga and others added 5 commits July 2, 2023 23:40
@gerben-stavenga @davidiw
first update
5bcb383
@gerben-stavenga @davidiw
Use RawTransactionWithData
8993f16
@gerben-stavenga @davidiw
fix all locations
564c64f
@davidiw
introduce the gas fee authenticator
153c0b3 
* Push the gas fee payer transaction authenticator throughout the code
* Start poking at the use of the bit to indicate gas fee payer
@davidiw
[fee-payer] plumb through the VM and framework
8035f1c
gerben-stavenga
gerben-stavenga commented Jul 3, 2023
View reviewed changes
Copy link
Contributor Author

@gerben-stavenga gerben-stavenga left a comment

Choose a reason for hiding this comment

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

lgtm

davidiw
davidiw approved these changes Jul 3, 2023
View reviewed changes
Copy link
Contributor

@davidiw davidiw left a comment

Choose a reason for hiding this comment

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

good with the few tweaks and of course cicd passing

aptos-move/aptos-vm/src/errors.rs Outdated Show resolved Hide resolved
ecosystem/indexer-grpc/indexer-grpc-parser/src/models/default_models/signatures.rs Outdated Show resolved Hide resolved
sdk/src/types.rs Outdated Show resolved Hide resolved
types/src/transaction/authenticator.rs Outdated Show resolved Hide resolved
@github-actions

This comment has been minimized.

Sign in to view
@github-actions

This comment has been minimized.

Sign in to view
@github-actions

This comment has been minimized.

Sign in to view
@github-actions

This comment has been minimized.

Sign in to view
@github-actions

This comment has been minimized.

Sign in to view
@github-actions

This comment has been minimized.

Sign in to view
@github-actions

This comment has been minimized.

Sign in to view
@github-actions

This comment has been minimized.

Sign in to view
@github-actions

This comment has been minimized.

Sign in to view
@github-actions

This comment has been minimized.

Sign in to view
@github-actions

This comment has been minimized.

Sign in to view
@github-actions

This comment has been minimized.

Sign in to view
@github-actions

This comment has been minimized.

Sign in to view
@github-actions

This comment has been minimized.

Sign in to view
@github-actions

This comment has been minimized.

Sign in to view
@github-actions

This comment has been minimized.

Sign in to view
@github-actions

This comment has been minimized.

Sign in to view
@github-actions

This comment has been minimized.

Sign in to view
@github-actions
Copy link
Contributor

github-actions bot commented Jul 4, 2023

✅ Forge suite compat success on aptos-node-v1.4.4 ==> b1f8bb8905d8d66a0f435f0518658e4fd8453e82

Compatibility test results for aptos-node-v1.4.4 ==> b1f8bb8905d8d66a0f435f0518658e4fd8453e82 (PR)
1. Check liveness of validators at old version: aptos-node-v1.4.4
compatibility::simple-validator-upgrade::liveness-check : committed: 7148 txn/s, latency: 4627 ms, (p50: 4500 ms, p90: 6600 ms, p99: 8100 ms), latency samples: 243040
2. Upgrading first Validator to new version: b1f8bb8905d8d66a0f435f0518658e4fd8453e82
compatibility::simple-validator-upgrade::single-validator-upgrade : committed: 4334 txn/s, latency: 7423 ms, (p50: 8200 ms, p90: 9400 ms, p99: 9600 ms), latency samples: 160360
3. Upgrading rest of first batch to new version: b1f8bb8905d8d66a0f435f0518658e4fd8453e82
compatibility::simple-validator-upgrade::half-validator-upgrade : committed: 4344 txn/s, latency: 7313 ms, (p50: 8300 ms, p90: 9100 ms, p99: 9700 ms), latency samples: 160760
4. upgrading second batch to new version: b1f8bb8905d8d66a0f435f0518658e4fd8453e82
compatibility::simple-validator-upgrade::rest-validator-upgrade : committed: 6233 txn/s, latency: 5276 ms, (p50: 5200 ms, p90: 8100 ms, p99: 9700 ms), latency samples: 218160
5. check swarm health
Compatibility test for aptos-node-v1.4.4 ==> b1f8bb8905d8d66a0f435f0518658e4fd8453e82 passed
Test Ok

@github-actions

This comment has been minimized.

Sign in to view
@github-actions

This comment has been minimized.

Sign in to view
@github-actions
Copy link
Contributor

github-actions bot commented Jul 4, 2023

✅ Forge suite realistic_env_max_load success on b1f8bb8905d8d66a0f435f0518658e4fd8453e82

two traffics test: inner traffic : committed: 6014 txn/s, latency: 6488 ms, (p50: 6300 ms, p90: 7800 ms, p99: 12000 ms), latency samples: 2610080
two traffics test : committed: 100 txn/s, latency: 2513 ms, (p50: 2400 ms, p90: 3000 ms, p99: 3600 ms), latency samples: 1800
Max round gap was 1 [limit 4] at version 1357841. Max no progress secs was 3.300517 [limit 10] at version 1357841.
Test Ok

@github-actions
Copy link
Contributor

github-actions bot commented Jul 4, 2023

✅ Forge suite framework_upgrade success on aptos-node-v1.4.4 ==> b1f8bb8905d8d66a0f435f0518658e4fd8453e82

Compatibility test results for aptos-node-v1.4.4 ==> b1f8bb8905d8d66a0f435f0518658e4fd8453e82 (PR)
Upgrade the nodes to version: b1f8bb8905d8d66a0f435f0518658e4fd8453e82
framework_upgrade::framework-upgrade::full-framework-upgrade : committed: 4416 txn/s, latency: 7363 ms, (p50: 7800 ms, p90: 10200 ms, p99: 16000 ms), latency samples: 167840
5. check swarm health
Compatibility test for aptos-node-v1.4.4 ==> b1f8bb8905d8d66a0f435f0518658e4fd8453e82 passed
Test Ok

@gerben-stavenga gerben-stavenga merged commit 604cb4f into main Jul 4, 2023
@gerben-stavenga gerben-stavenga deleted the gerben-fee-payer branch July 4, 2023 19:00
@gerben-stavenga gerben-stavenga linked an issue Jul 6, 2023 that may be closed by this pull request
[Feature Request] supporting paying gas on behalf of other users #6632
Closed
gedigi pushed a commit that referenced this pull request Aug 2, 2023
@gerben-stavenga @davidiw @gedigi
add gas fee payer by adding a new payload type (#8904)
f17b944 
* first update

* Use RawTransactionWithData

* fix all locations

* introduce the gas fee authenticator

* Push the gas fee payer transaction authenticator throughout the code
* Start poking at the use of the bit to indicate gas fee payer

* [fee-payer] plumb through the VM and framework

* move tests pass

* fix prover

* fix order

* regen

* Add

* restore sdk/src/types.rs

* remove old comment

---------

Co-authored-by: David Wolinsky <isaac.wolinsky@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gregnazario gregnazario gregnazario left review comments

@davidiw davidiw davidiw approved these changes

@movekevin movekevin movekevin approved these changes

@alinush alinush Awaiting requested review from alinush alinush is a code owner

@msmouse msmouse Awaiting requested review from msmouse msmouse is a code owner

@lightmark lightmark Awaiting requested review from lightmark lightmark is a code owner

@wrwg wrwg Awaiting requested review from wrwg wrwg is a code owner

@zekun000 zekun000 Awaiting requested review from zekun000 zekun000 is a code owner

@banool banool Awaiting requested review from banool banool is a code owner

@0xmaayan 0xmaayan Awaiting requested review from 0xmaayan 0xmaayan is a code owner

@saharct saharct Awaiting requested review from saharct

@junkil-park junkil-park Awaiting requested review from junkil-park junkil-park is a code owner

@0xmigo 0xmigo Awaiting requested review from 0xmigo

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Feature Request] supporting paying gas on behalf of other users
5 participants
@gerben-stavenga @davidiw @gregnazario @zekun000 @movekevin