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

Error in GetWithdrawalQueue and make bufgen Command Failure #84

Closed
ByungHeonLEE opened this issue Sep 5, 2024 · 3 comments · Fixed by #113
Closed

Error in GetWithdrawalQueue and make bufgen Command Failure #84

ByungHeonLEE opened this issue Sep 5, 2024 · 3 comments · Fixed by #113
Assignees
@zsystm
Labels
bug Something isn't working

Comments

@ByungHeonLEE
Copy link

ByungHeonLEE commented Sep 5, 2024 •
edited
Loading

Description and context

  • The GetWithdrawalQueue function is not functioning correctly due to un-marshalling fail
  • Additionally, the make bufgen command is not working as expected

Steps to reproduce

  1. Copy & Create grpc_query_test.go to client/x/evmstaking/keeper
  2. Run the test code
  3. Run themake bufgen to generate pb.go

Experienced behavior

  • The GetWithdrawalQueuefunction fails with a Protobuf error.

rpc error: code = Internal desc = proto: Withdrawal: illegal tag 0 (wire type 0)

  • The make bufgencommand does not run successfully

Solution recommendation

  • Debug the value(0x03), stored onWithdrawalQueueKeyprefix and make withdrawalsas proper protobuf
  • Update the buf_generate.shfile
@ByungHeonLEE ByungHeonLEE added the bug Something isn't working label Sep 5, 2024
@edisonz0718
Copy link
Contributor

What is the error when you run make bufgen ? It runs fine for me.

@zsystm
Copy link
Collaborator

zsystm commented Sep 6, 2024

@ByungHeonLEE
make bufgen would work if you install the following binaries and run again.

go install github.com/cosmos/cosmos-proto/cmd/protoc-gen-go-pulsar
go install github.com/cosmos/gogoproto/protoc-gen-cosmos
go install github.com/cosmos/gogoproto/protoc-gen-go-cosmos-orm
go install cosmossdk.io/orm/cmd/protoc-gen-go-cosmos-orm
go install google.golang.org/protobuf/cmd/protoc-gen-go

@edisonz0718
It would be great to add an auto-installation feature that checks for the required binaries needed to compile proto locally and installs them if they do not exist.

@edisonz0718
Copy link
Contributor

Yes, I think we have a separate make install-go-tools to install the dependency. We can check if the binaries exist and run this first before make bufgen

zsystm added a commit to zsystm/story that referenced this issue Sep 12, 2024
@zsystm
fix GetWithdrawalQueue
077ad02 
it should append QueueElementsPrefixSuffix when query elements of queue
it fixes piplabs#84
@zsystm zsystm mentioned this issue Sep 12, 2024
Merged
@zsystm zsystm self-assigned this Sep 12, 2024
zsystm added a commit to zsystm/story that referenced this issue Sep 20, 2024
@zsystm
fix GetWithdrawalQueue
dcb71c7 
it should append QueueElementsPrefixSuffix when query elements of queue
it fixes piplabs#84
@zsystm zsystm closed this as completed in ed00e1a Sep 20, 2024
leeren pushed a commit that referenced this issue Oct 4, 2024
@zsystm @leeren
fix(evmstaking): query withdrawal queue (#113)
3d79b4b 
it should append QueueElementsPrefixSuffix when query elements of queue
it fixes #84
leeren pushed a commit to leeren/story-fork that referenced this issue Oct 10, 2024
@zsystm @leeren
fix(evmstaking): query withdrawal queue (piplabs#113)
56c251c 
it should append QueueElementsPrefixSuffix when query elements of queue
it fixes piplabs#84
leeren pushed a commit to leeren/story-fork that referenced this issue Oct 10, 2024
@zsystm @leeren
fix(evmstaking): query withdrawal queue (piplabs#113)
155087c 
it should append QueueElementsPrefixSuffix when query elements of queue
it fixes piplabs#84
leeren pushed a commit to leeren/story-fork that referenced this issue Oct 10, 2024
@zsystm @leeren
fix(evmstaking): query withdrawal queue (piplabs#113)
96a3c83 
it should append QueueElementsPrefixSuffix when query elements of queue
it fixes piplabs#84
leeren pushed a commit to leeren/story-fork that referenced this issue Oct 10, 2024
@zsystm @leeren
fix(evmstaking): query withdrawal queue (piplabs#113)
9663276 
it should append QueueElementsPrefixSuffix when query elements of queue
it fixes piplabs#84
leeren pushed a commit that referenced this issue Oct 15, 2024
@zsystm @leeren
fix(evmstaking): query withdrawal queue (#113)
5f7695d 
it should append QueueElementsPrefixSuffix when query elements of queue
it fixes #84
leeren pushed a commit that referenced this issue Oct 15, 2024
@zsystm @leeren
fix(evmstaking): query withdrawal queue (#113)
64836a3 
it should append QueueElementsPrefixSuffix when query elements of queue
it fixes #84
leeren pushed a commit that referenced this issue Oct 15, 2024
@zsystm @leeren
fix(evmstaking): query withdrawal queue (#113)
e2d5560 
it should append QueueElementsPrefixSuffix when query elements of queue
it fixes #84
limengformal pushed a commit that referenced this issue Oct 15, 2024
@leeren @jdubpark
@zsystm @0xHansLee @jhd2best @edisonz0718 @ezreal1997 @Ramarti @dependabot @sebsadface
chore(release): merge iliad client changes to odyssey (#199)
f51e781 
* feat(x/evmstaking): max unbond withdrawal

* chore(x/evmstaking): bump log level

* feat(x/evmstaking): spendable coin max amount with tests

* feat(x/evmstaking): pass zero amount

* chore(x/evmstaking): move logs

* chore(release): finalize client 0.10.1 stable release

* chore(release): begin story v0.10.2 unstable release

* feat(cmd): one block rollback  (#157)

add cometBFT's one block rollback cmd

* chore(release): finalize client 0.10.2 stable release

* chore(release): begin client 0.11.0 unstable release

* fix(x/evmstaking): endblock unbond branch check (#163)

* fix(x/evmstaking): branch condition on max spendable

* test(x/evmstaking): add expected call in tests

* feat(cli): add unjail validator subcommand (#170)

* feat(cli): add unjail validator subcommand

* fix: use static predeploy ca in validator logic

* chore(release): finalize client 0.11.0 stable release

* chore(release): begin story client v0.12.0 unstable release

* feat(epochs): port epochs module from cosmos-sdk (#101)

* chore: modify path for mockgen (#121)

* chore(evmstaking): remove unnecessary codes (#125)

* add test cases for evmstaking/types/params

increased test coverage to 100%

changes
validate functions use concrete type instead of any. there is no reasoning for using any.

rename withdraw test suite to avoid name conflict with param test suite

* add test cases for evmstaking/keeper/unjail

increased test coverage to 100%

* add test cases for genesis

increased coverage to 71.4%

* fix syntax after rebase

* add test cases for genesis (#112)

increase coverage to 100%

* fix(evmstaking): query withdrawal queue (#113)

it should append QueueElementsPrefixSuffix when query elements of queue
it fixes #84

* test(evmengine): add test cases for db (#120)

* add test cases to db

* test(evmengine): add test cases for helper (#131)

* add test cases for helper

increased test coverage to 100%

* fix ci

* add test cases for keeper (#132)

increased test coverage to 89.7%

add ci rule to not use t.Parallel to avoid data race issue for cosmos orm table.

* test(evmengine): add test cases for genesis (#134)

increase coverage to 83.3%
moved common function to keeper_internal_test.go

* add test cases for genesis (#135)

* test(evmengine): add test cases for params (#136)

increased coverage to 77.3%

* add test cases for params (#137)

increased coverage to 100%

* feat(script): auto add binary version to s3 file (#142)

* add version txt

* fixed task name

* fixed url

* fixed url

* opti title

* finish test

* fixed platform

* fixed pre-commit

* fix(evmengine): nil panic with optimistic build enabled (#128)

cmtAPI is lazily set, so during replyBlocks it is nil.

* test(evmengine/keeper): add test cases for msg server (#100)

changes of mockEngineAPI
- forceInvalidNewPayloadV3 and forceInvalidForkchoiceUpdatedV3 are added to simulate failed apis

changes of engineMock
- Add storeKey to make engineMock's methods dependent on sdk.Context for better testability

NewBlock
- Because of above changes, we need to rlp encode and decode block data. But if we create a block with nil withdrawals and nil withdrawalHash, rlp doesn't work well (couldn't figure out the root cause)
- To avoid rlp error, pass non-nil withdrawals so withdrawalHash can be set as non-nil emptyHash value.

* test(evmengine): add test cases for abci (#143)

increased coverage to 83.8%
added mock getPayloadV3Func for mocking getPayloadV3

* feat(genesis): disable vote extension by default (#173)

* refactor(panic): remove unused or unnecessary panic code (#171)

* feat(mint): customized mint module (#169)

* feat(mint): port mint module from cosmos sdk v0.50.7

* feat(mint): port mint module from cosmos sdk v0.50.7

* feat(mint): port mint module from cosmos sdk v0.50.7

* feat(mint): port mint module from cosmos sdk v0.50.7

* feat(mint): remove useless parameters

* feat(mint): new inflation function

* feat(mint): mint parameters change event log processing

* feat(mint): mint module api

* feat(mint): placeholder for param change events

* feat(mint): fix unit test

* feat(mint): fix based on comments

* feat(mint): fix based on comments

* feat(mint): update readme

* feat(mint): update readme

* feat(netconf): fix genesis config of mint module in local netconf (#175)

* feat(contracts): transparent proxies and el genesis (#165)

* Change UUPS by TransparentUpgradeableProxy, use etching to generate initial contracts

* etch all the contracts

* git ignore local dumps

* fix contracts/script/EtchInitialState.s.sol, use it also as setup for tests

* fix GenerateAlloc for UpgradeEntryPoint, add allocations and test upgradeability

* lint and cl fixes

* temporarily disabled solhint in CI/CD

* temporarily remove run_solidity_lint

* temporarily remove lint form workflow

* remove need for env function in test

* build(deps): bump github.com/spf13/cobra from 1.8.0 to 1.8.1 (#19)

Bumps [github.com/spf13/cobra](https://github.com/spf13/cobra) from 1.8.0 to 1.8.1.
- [Release notes](https://github.com/spf13/cobra/releases)
- [Commits](spf13/cobra@v1.8.0...v1.8.1)

---
updated-dependencies:
- dependency-name: github.com/spf13/cobra
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Jongwon Park <[email protected]>

* feat(api): add epochs api (#176)

* feat(api): add epochs api

* feat(api): add epoch info api

* feat(mint): update mint param type (#182)

* feat(mint): update param type

* feat(mint): make param value consistent among example and tests

* feat(cli): add key conversion subcommand (#174)

* feat(cli): add key conversion subcommand

* adds uncompressed pubkey (hex) as input

* fix(CI/CD): fix solhint (#190)

* fix(solhint): rm redundant pre-commit linting hook

* fix(solhint): update linting script & pkg version

* chore(contracts): apply linting fixes

* chore(cmd): fix validator flag linting issue

---------

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: Jongwon Park <[email protected]>
Co-authored-by: zsystm <[email protected]>
Co-authored-by: Jongwon Park <[email protected]>
Co-authored-by: Narangde <[email protected]>
Co-authored-by: zsystm <[email protected]>
Co-authored-by: Haodi <[email protected]>
Co-authored-by: Ze <[email protected]>
Co-authored-by: Zerui Ge <[email protected]>
Co-authored-by: Ramarti <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Seb <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@zsystm zsystm

Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix(evmstaking): query withdrawal queue zsystm/story
3 participants
@edisonz0718 @ByungHeonLEE @zsystm