-
Notifications
You must be signed in to change notification settings - Fork 123
looprpc: fix several minor bugs #926
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
Conversation
3744dfb to
8a5e649
Compare
sputn1ck
left a comment
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.
LGTM, thanks for the fix
| The destination address of the withdrawal transaction. | ||
| */ | ||
| string pk_script = 2; | ||
| string address = 2; |
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.
just a note: this is a breaking change for json clients, but I don't think this is a problem.
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.
Same, I think it's not a problem atm.
bhandras
left a comment
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.
LGTM, thank you! 🎉
| rpc-check: rpc | ||
| @$(call print, "Verifying protos.") | ||
| if test -n "$$(git describe --dirty | grep dirty)"; then echo "Protos not properly formatted or not compiled with correct version!"; git status; git diff; exit 1; fi | ||
| if test -n "$$(git status --porcelain)"; then echo "Protos not properly formatted or not compiled with correct version!"; git status; git diff; exit 1; fi |
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.
💡
| The destination address of the withdrawal transaction. | ||
| */ | ||
| string pk_script = 2; | ||
| string address = 2; |
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.
Same, I think it's not a problem atm.
'git describe' may not work in CI because of Git shallow clones. We got false negative of this check: https://github.com/lightninglabs/loop/actions/runs/14556035205/job/40959214856 "fatal: No names found, cannot describe anything."
swapserverrpc/v1.0.11 was force-pushed causing build errors with GOPROXY=direct. Also updated in main go.mod, however it didn't fail, because replace to local dir is used.
GOPROXY=direct is needed to capture the issue with tags force-pushes.
It returns an address, but the field was called pk_script, which was confusing. Also renamed it in the code.
|
Rebased |
Renamed field
looprpc.WithdrawDepositsResponse.PkScripttoAddress, because it stores an address, not a pk_script.looprpc was not generated (
make rpc) and CI didn't catch this. Formatted and fixed CI.Also go.mod in
looprpcwas not working if GOPROXY=direct is used. Fixed and added a check to CI.Removed some unneeded renamings of swapserverrpc import to looprpc.
Pull Request Checklist
release_notes.mdif your PR contains major features, breaking changes or bugfixes