-
Notifications
You must be signed in to change notification settings - Fork 28
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
fix(ante): use native ante handler and check permissions for vesting URLs #197
fix(ante): use native ante handler and check permissions for vesting URLs #197
Conversation
app/ante/handlers.go
Outdated
ethante.NewEthSigVerificationDecorator(options.EvmKeeper), | ||
ethante.NewEthAccountVerificationDecorator(options.AccountKeeper, options.EvmKeeper), | ||
ethante.NewCanTransferDecorator(options.EvmKeeper), | ||
ethante.NewVirtualFrontierContractDecorator(options.EvmKeeper), // prevent transfer to virtual frontier contract |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NewVirtualFrontierContractDecorator
can be removed from rollapp-evm
vfc
is not wired in app.go
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok, are you sure?
just FYI though
it's here as well https://github.com/dymensionxyz/ethermint/blob/b0f9c1b0a37f6fd06d9dc61745ec1e9561461a29/app/ante/handler_options.go#L78
which is what we would depend on if we didn't implement our own
return sdk.ChainAnteDecorators( | ||
cosmosDecorators( | ||
options, | ||
func(c *codectypes.Any) bool { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's wrong, it supposed to be return false.
anyway I think it's more clear to leave it nil, and have the default (which is return false)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
but this is the legacy one
so we dont want to reject, because the legacy one HAS the extension
/ethermint.types.v1.ExtensionOptionsWeb3Tx
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it should be added here
ExtensionOptionChecker: evmostypes.HasDynamicFeeExtensionOption,
we will do this on 2.1x instead |
Description
see also #200
Closes #193
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow-up issues.
PR review checkboxes:
I have...
Unreleased
section inCHANGELOG.md
godoc
commentsSDK Checklist
map
time.Now()
sendCoin
and notSendCoins
Full security checklist here
----;
For Reviewer:
---;
After reviewer approval: