-
Notifications
You must be signed in to change notification settings - Fork 14
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(chainflip-broker-api): replace u128 with U256 #4656
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #4656 +/- ##
======================================
- Coverage 72% 72% -0%
======================================
Files 414 414
Lines 68457 68313 -144
Branches 68457 68313 -144
======================================
- Hits 49252 49054 -198
- Misses 16802 16855 +53
- Partials 2403 2404 +1 ☔ View full report in Codecov by Sentry. |
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.
Please use U128 or U256. We are trying to move away from NumberOrHex. In this case the encoding is the same, but not in all cases (which is why we should stop using NumberOrHex).
Also I believe this is a breaking change right? |
Actually I looked into it and the u128 will get serialized to a string. |
It won't be if it's included in 1.3 release |
…tation * origin/main: feat: support account deletion (#4314) fix: cf_pools_environment rpc encoding (#4674) fix: submission watcher could confuse/lose track of submissions (#4667) fix(github_actions/release_checks): update confusing runtime spec version check 🤦♂️ (#4672) feat: fix log messages, evm chain specific (#4652) fix: remove cfe events migration (#4671) chore: run CI on `nscloud` runners 🚀 (#4505) logging: lp-api panic in submission watcher (#4664) chore(localnet): append timestamp to log files 🪵 (#4660) fix(chainflip-broker-api): replace u128 with U256 (#4656) fix: remove unused cli command line options (#4644) chore: revert addition of cargo make (#4659) fix: correct cfe-events pallet version (#4658) chore: update systemd config to match 1.4 version 🚀 (#4655) fix: publish `chainflip-engine1.3` to debian packages 🐞 (#4653) Remove aptly check from publish workflow (#4650) fix: more try-runtime unwraps. (#4648) feat: RPC for returning scale-encoded System events (#4638)
Pull Request
Checklist
Please conduct a thorough self-review before opening the PR.
Summary
For consistency, this should return
NumberOrHex
instead ofu128
.u128
is too large to be safely represented in JSON, but in this case there shouldn't be any issues because the fee shouldn't exceed 2^53 - 1 (at least immediately).