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

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

Merged
merged 4 commits into from
Oct 10, 2023
Merged

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

merged 4 commits into from
Oct 10, 2023

Conversation

lumtis
Copy link
Member

@lumtis lumtis commented Oct 9, 2023 •
edited

Description

Update UpdateZRC20WithdrawFee to add the ability to update the gas limit of a zrc20 along with the protocol fee.
If one of the value is not defined, the value is not updated for the zrc20

Closes: #1256

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

@lumtis lumtis self-assigned this Oct 9, 2023
@lumtis lumtis marked this pull request as ready for review October 9, 2023 22:22
brewmaster012
brewmaster012 approved these changes Oct 10, 2023
View reviewed changes
Copy link
Collaborator

@brewmaster012 brewmaster012 left a comment

Choose a reason for hiding this comment

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

LGTM
seems a more appropriate name for the MsgUpdateZRC20WithdrawFee is needed
could be in another PR though.

@lumtis
Copy link
Member Author

lumtis commented Oct 10, 2023

LGTM seems a more appropriate name for the MsgUpdateZRC20WithdrawFee is needed could be in another PR though.

ProtocolFee and GasLimit are both used to calculate the final withdraw fee, that's why I was thinking this is convenable but I agree we can think about another name

ws4charlie
ws4charlie approved these changes Oct 10, 2023
View reviewed changes
Copy link
Contributor

@ws4charlie ws4charlie left a comment

Choose a reason for hiding this comment

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

looks good

@lumtis lumtis merged commit ed35305 into develop Oct 10, 2023
13 checks passed
@lumtis lumtis deleted the feat/msg-gas-limit branch October 10, 2023 20:58
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 <88689859+brewmaster012@users.noreply.github.com>

* 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 <charliec@zetachain.com>

* 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 <charliec@zetachain.com>

* 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 <lucas.bertrand.22@gmail.com>

* code adjustment according to feedback of PR review

* corrected a typo and added more comment to function

* fix gosec error

---------

Co-authored-by: charliec <charliec@zetachain.com>
Co-authored-by: Lucas Bertrand <lucas.bertrand.22@gmail.com>
Co-authored-by: brewmaster012 <88689859+brewmaster012@users.noreply.github.com>

* 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 <lucas.bertrand.22@gmail.com>
Co-authored-by: brewmaster012 <88689859+brewmaster012@users.noreply.github.com>
Co-authored-by: kevinssgh <79858682+kevinssgh@users.noreply.github.com>
Co-authored-by: charliec <charliec@zetachain.com>
Co-authored-by: Tanmay <tanmay@zetachain.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ws4charlie ws4charlie ws4charlie approved these changes

@brewmaster012 brewmaster012 brewmaster012 approved these changes

@kingpinXD kingpinXD Awaiting requested review from kingpinXD kingpinXD is a code owner

@kevinssgh kevinssgh Awaiting requested review from kevinssgh

@fadeev fadeev Awaiting requested review from fadeev

Assignees

@lumtis lumtis

Labels
breaking:proto
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

add message to update ZRC20 GasLimit
3 participants
@lumtis @ws4charlie @brewmaster012