Skip to content

Conversation

@syntrust
Copy link

@syntrust syntrust commented Jul 16, 2025

Failed tests:

golangci-lint run -E goimports,sqlclosecheck,bodyclose,asciicheck,misspell,errorlint --timeout 5m -e "errors.As" -e "errors.Is" ./...
op-chain-ops/cmd/check-fjord/checks/checks.go:1: : # github.com/ethereum-optimism/optimism/op-chain-ops/cmd/check-fjord/checks
op-chain-ops/cmd/check-fjord/checks/checks.go:286:3: not enough arguments in call to types.NewL1CostFuncFjord
        have (*big.Int, *big.Int, *big.Int, *big.Int)
        want (*big.Int, *big.Int, *big.Int, *big.Int, *big.Int, *big.Int) (typecheck)
package checks
op-chain-ops/cmd/check-fjord/main.go:9:2: could not import github.com/ethereum-optimism/optimism/op-chain-ops/cmd/check-fjord/checks (-: # github.com/ethereum-optimism/optimism/op-chain-ops/cmd/check-fjord/checks
op-chain-ops/cmd/check-fjord/checks/checks.go:286:3: not enough arguments in call to types.NewL1CostFuncFjord
        have (*big.Int, *big.Int, *big.Int, *big.Int)
        want (*big.Int, *big.Int, *big.Int, *big.Int, *big.Int, *big.Int)) (typecheck)
        "github.com/ethereum-optimism/optimism/op-chain-ops/cmd/check-fjord/checks"
        ^
op-acceptance-tests/tests/fjord/check_scripts_test.go:11:14: could not import github.com/ethereum-optimism/optimism/op-chain-ops/cmd/check-fjord/checks (-: # github.com/ethereum-optimism/optimism/op-chain-ops/cmd/check-fjord/checks
op-chain-ops/cmd/check-fjord/checks/checks.go:286:3: not enough arguments in call to types.NewL1CostFuncFjord
        have (*big.Int, *big.Int, *big.Int, *big.Int)
        want (*big.Int, *big.Int, *big.Int, *big.Int, *big.Int, *big.Int)) (typecheck)
        fjordChecks "github.com/ethereum-optimism/optimism/op-chain-ops/cmd/check-fjord/checks"
                    ^
op-e2e/actions/proofs/fixtures.go:1: : # github.com/ethereum-optimism/optimism/op-e2e/actions/proofs [github.com/ethereum-optimism/optimism/op-e2e/actions/proofs.test]
op-e2e/actions/proofs/operator_fee_test.go:394:3: not enough arguments in call to types.NewL1CostFuncFjord
        have (*big.Int, *big.Int, *big.Int, *big.Int)
        want (*big.Int, *big.Int, *big.Int, *big.Int, *big.Int, *big.Int) (typecheck)
package proofs
op-e2e/actions/upgrades/dencun_fork_test.go:1: : # github.com/ethereum-optimism/optimism/op-e2e/actions/upgrades [github.com/ethereum-optimism/optimism/op-e2e/actions/upgrades.test]
op-e2e/actions/upgrades/fjord_fork_test.go:145:3: not enough arguments in call to types.NewL1CostFuncFjord
        have (*big.Int, *big.Int, *big.Int, *big.Int)
        want (*big.Int, *big.Int, *big.Int, *big.Int, *big.Int, *big.Int) (typecheck)
package upgrades
op-e2e/system/fjord/check_scripts_test.go:18:14: could not import github.com/ethereum-optimism/optimism/op-chain-ops/cmd/check-fjord/checks (-: # github.com/ethereum-optimism/optimism/op-chain-ops/cmd/check-fjord/checks
op-chain-ops/cmd/check-fjord/checks/checks.go:286:3: not enough arguments in call to types.NewL1CostFuncFjord
        have (*big.Int, *big.Int, *big.Int, *big.Int)
        want (*big.Int, *big.Int, *big.Int, *big.Int, *big.Int, *big.Int)) (typecheck)
        fjordChecks "github.com/ethereum-optimism/optimism/op-chain-ops/cmd/check-fjord/checks"
                    ^
make: *** [Makefile:24: lint-go] Error 1

Verified with:

golangci-lint run -E goimports,sqlclosecheck,bodyclose,asciicheck,misspell,errorlint --timeout 5m -e "errors.As" -e "errors.Is" ./...
# or 
make lint-go

@syntrust syntrust marked this pull request as ready for review July 16, 2025 10:31
}

func fjordL1Cost(l1BlockInfo *derive.L1BlockInfo, rollupCostData types.RollupCostData) *big.Int {
func fjordL1Cost(config *genesis.DeployConfig, l1BlockInfo *derive.L1BlockInfo, rollupCostData types.RollupCostData) *big.Int {

Choose a reason for hiding this comment

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

To minimize the diff, we can just pass 1 as the multipliers for op tests?

Copy link
Author

Choose a reason for hiding this comment

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

If the multipliers are configurable, we may need to read them from the configuration. Otherwise, it is meaningless to run the check.

Choose a reason for hiding this comment

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

Currently these multipliers can only be upgraded by upgrade of op-geth.

@blockchaindevsh
Copy link

The changes for check-fjord looks valid, but I'm not sure whether we're going to use the tool. If not, we can just pass 1 for the multipliers.

@syntrust
Copy link
Author

The changes for check-fjord looks valid, but I'm not sure whether we're going to use the tool. If not, we can just pass 1 for the multipliers.

I guess that depends on our main goal—do we need to keep the tool up to date, simply pass go lint, or minimize the diff?

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.

3 participants