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.

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

[Aptos Framework] Add check on empty vector during removed multisig v2 owners event emission #8448

Merged
merged 1 commit into from May 31, 2023

Conversation

alnoki
Copy link
Contributor

@alnoki alnoki commented May 31, 2023

@davidiw @movekevin @wrwg

Presently remove_owners accepts an owners_to_remove vector that does not necessarily have any overlap with existing signatories. Hence it is possible to successfully invoke the removal operation without actually removing any owners, resulting in an empty event emission. To avoid the confusion of empty events, this PR adds a check against a correspondingly empty vector.

Pending the resolution of #5369 there is no way to test this logic (in a unit test context), which may explain why it went unnoticed until now.

@davidiw davidiw enabled auto-merge (squash) May 31, 2023 16:29
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions
Copy link
Contributor

✅ Forge suite compat success on testnet_2d8b1b57553d869190f61df1aaf7f31a8fc19a7b ==> 584c603edd1b27bf6c78aeca9502cbcc19e7256d

Compatibility test results for testnet_2d8b1b57553d869190f61df1aaf7f31a8fc19a7b ==> 584c603edd1b27bf6c78aeca9502cbcc19e7256d (PR)
1. Check liveness of validators at old version: testnet_2d8b1b57553d869190f61df1aaf7f31a8fc19a7b
compatibility::simple-validator-upgrade::liveness-check : 10079 TPS, 3800 ms latency, 6800 ms p99 latency,no expired txns
2. Upgrading first Validator to new version: 584c603edd1b27bf6c78aeca9502cbcc19e7256d
compatibility::simple-validator-upgrade::single-validator-upgrade : 6090 TPS, 6644 ms latency, 9200 ms p99 latency,no expired txns
3. Upgrading rest of first batch to new version: 584c603edd1b27bf6c78aeca9502cbcc19e7256d
compatibility::simple-validator-upgrade::half-validator-upgrade : 5962 TPS, 6616 ms latency, 8500 ms p99 latency,no expired txns
4. upgrading second batch to new version: 584c603edd1b27bf6c78aeca9502cbcc19e7256d
compatibility::simple-validator-upgrade::rest-validator-upgrade : 7901 TPS, 4797 ms latency, 8200 ms p99 latency,no expired txns
5. check swarm health
Compatibility test for testnet_2d8b1b57553d869190f61df1aaf7f31a8fc19a7b ==> 584c603edd1b27bf6c78aeca9502cbcc19e7256d passed
Test Ok

@github-actions
Copy link
Contributor

✅ Forge suite land_blocking success on 584c603edd1b27bf6c78aeca9502cbcc19e7256d

performance benchmark : 5747 TPS, 6890 ms latency, 26800 ms p99 latency,no expired txns
Test Ok

@github-actions
Copy link
Contributor

✅ Forge suite framework_upgrade success on aptos-node-v1.3.0_3fc3d42b6cfe27460004f9a0326451bcda840a60 ==> 584c603edd1b27bf6c78aeca9502cbcc19e7256d

Compatibility test results for aptos-node-v1.3.0_3fc3d42b6cfe27460004f9a0326451bcda840a60 ==> 584c603edd1b27bf6c78aeca9502cbcc19e7256d (PR)
Upgrade the nodes to version: 584c603edd1b27bf6c78aeca9502cbcc19e7256d
framework_upgrade::framework-upgrade::full-framework-upgrade : 6381 TPS, 6265 ms latency, 13200 ms p99 latency,no expired txns
5. check swarm health
Compatibility test for aptos-node-v1.3.0_3fc3d42b6cfe27460004f9a0326451bcda840a60 ==> 584c603edd1b27bf6c78aeca9502cbcc19e7256d passed
Test Ok

@davidiw davidiw merged commit 3fe9436 into aptos-labs:main May 31, 2023
57 of 58 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants