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

SGT: add E2E test -es #125

Merged
merged 7 commits into from
Dec 20, 2024
Merged

SGT: add E2E test -es #125

merged 7 commits into from
Dec 20, 2024

Conversation

dajuguan
Copy link

@dajuguan dajuguan commented Dec 12, 2024

Description

This PR address the issue(#101) of SGT's E2E tests

ABI and go bindings generation

cd packages/contracts-bedrock
forge inspect src/L2/SoulGasToken.sol:SoulGasToken abi>>SoulGasToken.json
mv SoulGasToken.json snapshots/abi/

cd op-e2e
abigen --abi ../packages/contracts-bedrock/snapshots/abi/SoulGasToken.json --pkg bindings --type SoulGasToken -out bindings/SoulGasToken.go

Test

cd op-e2e
make devnet-allocs
go test -run '^(TestSGTDepositSuccess|TestSGTAsGasPayment)$' github.com/ethereum-optimism/optimism/op-e2e/sgt  -count=1 -v

Matched test cases in original opgeth_test.go

  • no SoulGasToken: nativaGasPaymentWithoutSGTSuccess
  • have SoulGasToken but not enough: fullSGTInsufficientGasPaymentFail
  • insufficient native token:
    • fullSGTGasPaymentAndNonZeroTxValueWithInsufficientNativeBalanceFail
    • partialSGTGasPaymentAndNonZeroTxValueWithInsufficientNativeBalanceFail
  • cost exceed native+sgt balance: partialSGTInsufficientGasPaymentFail
  • happy path:
    • nativaGasPaymentWithoutSGTSuccess
    • fullSGTGasPaymentWithoutNativeBalanceSuccess
    • fullSGTGasPaymentWithNativeBalanceSuccess
    • partialSGTGasPaymentSuccess
    • fullSGTGasPaymentAndNonZeroTxValueWithSufficientNativeBalanceSuccess
    • partialSGTGasPaymentAndNonZeroTxValueWithSufficientNativeBalanceSuccess

Matched test cases in https://perfect-amphibian-929.notion.site/SWC-Devnet-1282e493e88380648486d4961eb91a8d

  • Deposit SGT for another user: TestSGTDepositFunctionSuccess
  • Spend SGT without native gas token: FullSGTGasPaymentWithoutNativeBalanceSuccess
  • Spend SGT with native gas token: PartialSGTGasPaymentSuccess

Out of the scope:

  • fullSGTInsufficientGasPaymentFail
  • fullNativeInsufficientGasPaymentFail

@dajuguan dajuguan force-pushed the po/sgt_e2e branch 4 times, most recently from bb5c052 to 92c2d68 Compare December 12, 2024 08:43
@blockchaindevsh
Copy link
Collaborator

For the record, could you also update the PR with the commands you run to create the auto-generated files?

@dajuguan
Copy link
Author

For the record, could you also update the PR with the commands you run to create the auto-generated files?

Done

op-e2e/sgt/sgt_test.go Outdated Show resolved Hide resolved
op-e2e/sgt/sgt_test.go Show resolved Hide resolved
@dajuguan dajuguan force-pushed the po/sgt_e2e branch 2 times, most recently from 0316b17 to 52db68f Compare December 13, 2024 17:11
@blockchaindevsh
Copy link
Collaborator

blockchaindevsh commented Dec 14, 2024

Could you update the PR description with additional information to aid review:

  1. which test in original opgeth_test.go corresponds to which action
  2. which test corresponds to the manual one here
  3. is there a test out of the scope above?

@dajuguan
Copy link
Author

Could you update the PR description with additional information to aid review:

  1. which test in original opgeth_test.go corresponds to which action
  2. which test corresponds to the manual one here
  3. is there a test out of the scope above?

Done

Copy link
Collaborator

@blockchaindevsh blockchaindevsh left a comment

Choose a reason for hiding this comment

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

Left some more comments, otherwise LGTM!

Copy link

@qzhodl qzhodl left a comment

Choose a reason for hiding this comment

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

LGTM

@blockchaindevsh blockchaindevsh merged commit fb37999 into op-es Dec 20, 2024
@blockchaindevsh blockchaindevsh deleted the po/sgt_e2e branch December 20, 2024 09:50
@@ -8,7 +8,8 @@
}
],
"stateMutability": "nonpayable",
"type": "constructor"
"type": "

Choose a reason for hiding this comment

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

Looks like this change breaks snapshots-check-no-build

Copy link
Author

Choose a reason for hiding this comment

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

Does the recent op-es commit pass the snapshots-check-no-build? I just accepted SoulGasToken.json file in op-es. I'll also double check that.

@dajuguan dajuguan mentioned this pull request Dec 22, 2024
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.

4 participants