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.

Sign up for GitHub

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

Jump to bottom

fix: read gas limit from smart contract #1277

Merged
merged 3 commits into from
Oct 12, 2023
Merged

fix: read gas limit from smart contract #1277

merged 3 commits into from
Oct 12, 2023

Conversation

lumtis
Copy link
Member

@lumtis lumtis commented Oct 11, 2023

Description

  • Query gas limit from smart contract for zrc20 withdraw in x/crosschain/keeper/evm_hooks.go
  • Setup gas token with the provided gas limit in the message instead of default 21000, 100000 in SetupChainGasCoinAndPool
  • Add some checks

Closes: #1276

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Include instructions and any relevant details so others can reproduce.

  • Tested CCTX in localnet
  • Tested in development environment
  • Go unit tests
  • Go integration tests
  • Tested via GitHub Actions

Checklist:

  • I have added unit tests that prove my fix feature works

@github-actions
Copy link

!!!WARNING!!!
nosec detected in the following files: x/fungible/keeper/msg_server_deploy_fungible_coin_zrc20.go

Be very careful about using #nosec in code. It can be a quick way to suppress security warnings and move forward with development, it should be employed with caution. Suppressing warnings with #nosec can hide potentially serious vulnerabilities. Only use #nosec when you're absolutely certain that the security issue is either a false positive or has been mitigated in another way.

Only suppress a single rule (or a specific set of rules) within a section of code, while continuing to scan for other problems. To do this, you can list the rule(s) to be suppressed within the #nosec annotation, e.g: /* #nosec G401 */ or //#nosec G201 G202 G203
Broad #nosec annotations should be avoided, as they can hide other vulnerabilities. The CI will block you from merging this PR until you remove #nosec annotations that do not target specific rules.

Pay extra attention to the way #nosec is being used in the files listed above.

@github-actions github-actions bot added the nosec label Oct 11, 2023
@lumtis lumtis self-assigned this Oct 11, 2023
@lumtis lumtis merged commit 48f3a44 into develop Oct 12, 2023
11 checks passed
@lumtis lumtis deleted the fix/gas-limit-withdraw branch October 12, 2023 20:36
ws4charlie added a commit that referenced this pull request Oct 13, 2023
@ws4charlie @lumtis
@brewmaster012 @kevinssgh @kingpinXD
fix: merge develop into inbound-tracker and unified proof verification (
1ab1338 
#1292)

* refactor: change default mempool version in config (#1238)

* fix(`MsgWhitelistERC20`): set unique index for generate cctx (#1245)

* update index

* remove deprecated functions

* make generate

* add return value in message

* initialize test

* whitelist tests

* make generate

* fix(`observer`): remove error return in `IsAuthorized` (#1250)

* update function

* regenerate interfaces

* update for crosschain

* fix(`GetForeignCoinFromAsset`): Ethereum comparaison checksum/non-checksum format (#1261)

* fix error message

* compare with ETH address type

* tests

* goimport

---------

Co-authored-by: brewmaster012 <[email protected]>

* feat(`fungible`): add ability to update gas limit (#1260)

* add new field

* update message type

* message new logic

* fix: Blame index update (#1264)

* initial commit

* added queries and unit tests

* added cli

* fix parse error

* fix parse error 2

* fix lint and test errors

* ran make generate

* update index for keygen

* refactor query name

* refactor key calculation

* refactor lib name

* fix: feed sataoshi/B to zetacore and check actual outTx size (#1243)

* feed sataoshi/B to zetacore and check size limit

* removed fee cap

* replaced magic number 1000 with constant bytesPerKB

* put lowerbound, upperbound limit on sizeLimit

* use actual txSize for fee calculation

---------

Co-authored-by: charliec <[email protected]>

* fix: cherry pick all hotfix from v10.0.x (zero-amount, precision, etc.) (#1235)

* cherry pick all hotfix from v10.0.x

* adjusted code to for nosec

* adusted error handling and code comments according to PR review feedback

* cherry pick hotfix for bitcoin outbound performance and updated some log prints

* cherry pick mock mainnet hotfix for duplicate payment on nonce 0

---------

Co-authored-by: charliec <[email protected]>

* fix: register emissions grpc server (#1257)

* feat: Bitcoin block header and merkle proof (#1263)

* initiated bitcoin header and proof

* added smoke test for bitcoin merkle proof and RPC query

* make generate

* fix gosec and unit test

* Update common/headers_test.go

Co-authored-by: Lucas Bertrand <[email protected]>

* code adjustment according to feedback of PR review

* corrected a typo and added more comment to function

* fix gosec error

---------

Co-authored-by: charliec <[email protected]>
Co-authored-by: Lucas Bertrand <[email protected]>
Co-authored-by: brewmaster012 <[email protected]>

* fix: read gas limit from smart contract (#1277)

* read gas limit from smart contract

* add more checks for gas limit

* fix(`fungible`): add CLI command to query system contract (#1252)

* fix proto

* fix filename

* add cli query

* fix(`cmd`): add notice when using `--ledger` with Ethereum HD path (#1285)

* change comment

* add notice for ledger

* merge develop into inbound-tracker and unified proof verification

* fixed gosec errors

---------

Co-authored-by: Lucas Bertrand <[email protected]>
Co-authored-by: brewmaster012 <[email protected]>
Co-authored-by: kevinssgh <[email protected]>
Co-authored-by: charliec <[email protected]>
Co-authored-by: Tanmay <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ws4charlie ws4charlie ws4charlie left review comments

@brewmaster012 brewmaster012 brewmaster012 approved these changes

@kingpinXD kingpinXD kingpinXD approved these changes

@kevinssgh kevinssgh Awaiting requested review from kevinssgh

@fadeev fadeev Awaiting requested review from fadeev

Assignees

@lumtis lumtis

Labels
nosec
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Unify gas limit logic
4 participants
@lumtis @kingpinXD @ws4charlie @brewmaster012