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

feat: apply gateway lookback limit to eth API lookback #10467

Merged
merged 3 commits into from
Mar 16, 2023

Conversation

Stebalien
Copy link
Member

Related Issues

fixes #10412

Proposed Changes

This change applies the gateway lookback limits to the eth txn and receipt lookup APIs. It:

  1. Introduces new "limited" API endpoints for EthGetTransactionByHash and EthGetTransactionReceipt that accept lookback-limits.
  2. Implements the gateway version of these API endpoints by calling the limited variants with the default message search lookback limit.

Checklist

Before you mark the PR ready for review, please make sure that:

  • Commits have a clear commit message.
  • PR title is in the form of of <PR type>: <area>: <change being made>
    • example: fix: mempool: Introduce a cache for valid signatures
    • PR type: fix, feat, build, chore, ci, docs, perf, refactor, revert, style, test
    • area, e.g. api, chain, state, market, mempool, multisig, networking, paych, proving, sealing, wallet, deps
  • New features have usage guidelines and / or documentation updates in
  • Tests exist for new functionality or change in behavior
  • CI is green

@Stebalien Stebalien requested a review from a team as a code owner March 14, 2023 05:02
@magik6k
Copy link
Contributor

magik6k commented Mar 14, 2023

Gateway tests are angry

--- FAIL: TestGatewayWalletMsig (11.08s)
panic: reflect.Set: value of type *api.Gateway is not assignable to type full.EthModuleAPI [recovered]
	panic: reflect.Set: value of type *api.Gateway is not assignable to type full.EthModuleAPI

goroutine 71 [running]:
testing.tRunner.func1.2({0x447bb40, 0xc00c58fdf0})
	/usr/local/go/src/testing/testing.go:1389 +0x24e
testing.tRunner.func1()
	/usr/local/go/src/testing/testing.go:1392 +0x39f
panic({0x447bb40, 0xc00c58fdf0})
	/usr/local/go/src/runtime/panic.go:838 +0x207
reflect.Value.assignTo({0x43d1520?, 0xc00c58fde0?, 0xc00111d998?}, {0x4a82910, 0xb}, 0x4915b20, 0xc00c58fdd0)
	/usr/local/go/src/reflect/value.go:3064 +0x2ac
reflect.Value.Set({0x4915b20?, 0xc00c58fdd0?, 0x4915b20?}, {0x43d1520?, 0xc00c58fde0?, 0x1?})
	/usr/local/go/src/reflect/value.go:2090 +0xeb
github.com/filecoin-project/lotus/node.as.func2({0xc00679ef48?, 0x2?, 0x2?})
	/home/circleci/lotus/node/options.go:156 +0x233
reflect.Value.call({0xc00a0bde00?, 0xc00d04f200?, 0x7574e5?}, {0x4a4cb74, 0x4}, {0xc00679ef30, 0x1, 0x30?})
	/usr/local/go/src/reflect/value.go:556 +0x845
reflect.Value.Call({0xc00a0bde00?, 0xc00d04f200?, 0x757847?}, {0xc00679ef30, 0x1, 0x1})
	/usr/local/go/src/reflect/value.go:339 +0xbf
go.uber.org/dig.defaultInvoker({0xc00a0bde00?, 0xc00d04f200?, 0xc00d408f60?}, {0xc00679ef30?, 0x1?, 0x59970b0?})
	/home/circleci/go/pkg/mod/go.uber.org/dig@v1.12.0/dig.go:355 +0x28
go.uber.org/dig.(*node).Call(0xc00d403180, {0x59970b0?, 0xc00d01b7c0})
	/home/circleci/go/pkg/mod/go.uber.org/dig@v1.12.0/dig.go:806 +0x259
go.uber.org/dig.paramSingle.Build({{0x0, 0x0}, 0x0, {0x59a7c48, 0x4915b20}}, {0x59970b0, 0xc00d01b7c0})
	/home/circleci/go/pkg/mod/go.uber.org/dig@v1.12.0/param.go:245 +0x242
go.uber.org/dig.paramObjectField.Build(...)
	/home/circleci/go/pkg/mod/go.uber.org/dig@v1.12.0/param.go:401
go.uber.org/dig.paramObject.Build({{0x59a7c48, 0x49f16a0}, {0xc000192fa0, 0x3, 0x4}}, {0x59970b0, 0xc00d01b7c0})
	/home/circleci/go/pkg/mod/go.uber.org/dig@v1.12.0/param.go:328 +0x148
go.uber.org/dig.paramObjectField.Build(...)
	/home/circleci/go/pkg/mod/go.uber.org/dig@v1.12.0/param.go:401
go.uber.org/dig.paramObject.Build({{0x59a7c48, 0x4a20600}, {0xc00d3fd180, 0x10, 0x10}}, {0x59970b0, 0xc00d01b7c0})
	/home/circleci/go/pkg/mod/go.uber.org/dig@v1.12.0/param.go:328 +0x148
go.uber.org/dig.paramList.BuildList({{0x59a7c48, 0xc000156300}, {0xc00e71dea0, 0x1, 0x1}}, {0x59970b0, 0xc00d01b7c0})
	/home/circleci/go/pkg/mod/go.uber.org/dig@v1.12.0/param.go:201 +0xb9
go.uber.org/dig.(*Container).Invoke(0xc00d01b7c0, {0xc000156300?, 0xc00d02e300}, {0x1b5665a?, 0x1?, 0x1?})
	/home/circleci/go/pkg/mod/go.uber.org/dig@v1.12.0/dig.go:503 +0x2b9
go.uber.org/fx.(*App).executeInvoke(0xc00d07ec30, {{0xc000156300, 0xc00d02e300}, {0xc00d022c80, 0x7, 0x8}})
	/home/circleci/go/pkg/mod/go.uber.org/fx@v1.15.0/app.go:964 +0x39f
go.uber.org/fx.(*App).executeInvokes(...)
	/home/circleci/go/pkg/mod/go.uber.org/fx@v1.15.0/app.go:929
go.uber.org/fx.New({0xc00111f2f8, 0x3, 0x1e?})
	/home/circleci/go/pkg/mod/go.uber.org/fx@v1.15.0/app.go:596 +0xa4b
github.com/filecoin-project/lotus/node.New({0x5983fb0, 0xc00005a068}, {0xc0002741c0, 0xb, 0xe})
	/home/circleci/lotus/node/builder.go:367 +0x477
github.com/filecoin-project/lotus/itests/kit.(*Ensemble).Start(0xc0006d6240)
	/home/circleci/lotus/itests/kit/ensemble.go:456 +0xd3d
command-line-arguments.startNodes({0x5983fb0, 0xc00005a068}, 0xc000703040, 0x8725a0?, 0x0?, 0xc0002765f0?)
	/home/circleci/lotus/itests/gateway_test.go:314 +0x39d
command-line-arguments.TestGatewayWalletMsig(0xd40010?)
	/home/circleci/lotus/itests/gateway_test.go:52 +0x70
testing.tRunner(0xc000703040, 0x55ed0c0)
	/usr/local/go/src/testing/testing.go:1439 +0x102
created by testing.(*T).Run
	/usr/local/go/src/testing/testing.go:1486 +0x35f

@@ -53,10 +53,12 @@ type EthModuleAPI interface {
EthGetBlockByHash(ctx context.Context, blkHash ethtypes.EthHash, fullTxInfo bool) (ethtypes.EthBlock, error)
EthGetBlockByNumber(ctx context.Context, blkNum string, fullTxInfo bool) (ethtypes.EthBlock, error)
EthGetTransactionByHash(ctx context.Context, txHash *ethtypes.EthHash) (*ethtypes.EthTx, error)
EthGetTransactionByHashLimited(ctx context.Context, txHash *ethtypes.EthHash, limit abi.ChainEpoch) (*ethtypes.EthTx, error)
Copy link
Contributor

Choose a reason for hiding this comment

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

Limited stuff probably shouldn't be on EthModule

EthModule / other API modules is what is replaced with a Gateway api when running in lite mode.

If we do want to provide limited APIs in lite mode, we will also need to put them on the Gateway api (I don't think it's needed for lite nodes, those most of the time will likely not be used for eth rpc stuff I imagine)

Copy link
Contributor

@arajasek arajasek left a comment

Choose a reason for hiding this comment

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

The limits aren't actually enforced?

node/impl/full/eth.go Outdated Show resolved Hide resolved
@magik6k magik6k requested a review from arajasek March 15, 2023 09:10
This change:

1. Introduces new "limited" API endpoints for EthGetTransactionByHash
   and EthGetTransactionReceipt that accept lookback-limits.
2. Implements the gateway version of these API endpoints by calling the
   limited variants with the default message search lookback limit.

fixes #10412
@jennijuju jennijuju added this to the v1.21.0 milestone Mar 16, 2023
@magik6k magik6k merged commit 093ff95 into master Mar 16, 2023
@magik6k magik6k deleted the steb/gw-eth-lookback-limit branch March 16, 2023 07:51
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.

Gateway: EthAPI needs to restrict lookbacks
4 participants