Skip to content

Dynamic loan contract transaction input index#42

Closed
da-kami wants to merge 1 commit into
comit-network:masterfrom
da-kami:fix-loan-repay
Closed

Dynamic loan contract transaction input index#42
da-kami wants to merge 1 commit into
comit-network:masterfrom
da-kami:fix-loan-repay

Conversation

@da-kami
Copy link
Copy Markdown
Member

@da-kami da-kami commented Aug 3, 2021

Given that we can have multiple txins the loan contract might not always be at index position 1.
Since we push the loan contract txin into the vec as last element we can be sure it is the last element in the vec when we satisfy the loan repayment.

Thanks to @thomaseizinger and @bonomat for the debugging help :)

Given that we can have multiple txins the loan contract might not always be at index position 1.
Since we push the loan contract txin into the vec as last element we can be sure it is the last element in the vec when we satisfy the loan repayment.
@da-kami
Copy link
Copy Markdown
Member Author

da-kami commented Aug 3, 2021

@luckysori it would be great if you could add a test that ensures the fix. I tested with waves UI and did not encounter repayment issues anymore.

@luckysori
Copy link
Copy Markdown
Collaborator

Thank you for the bug fix!

@luckysori it would be great if you could add a test that ensures the fix. I tested with waves UI and did not encounter repayment issues anymore.

Shall we merge this as is and record an issue? We're overhauling the tests with #34, so it will be easier and less expensive to test corner cases like this one once that's merged.

@luckysori
Copy link
Copy Markdown
Collaborator

Is there any way that I can take over a PR from a fork? @da-kami @thomaseizinger

That way I can complete the rebase and add an entry to the changelog, and you don't have to worry about it.

@thomaseizinger
Copy link
Copy Markdown
Contributor

Is there any way that I can take over a PR from a fork? @da-kami @thomaseizinger

That way I can complete the rebase and add an entry to the changelog, and you don't have to worry about it.

Yes, if @da-kami has the checkbox at the right end of the PR checked (which it is by default), you can checkout a PR locally and continue developing.

I'd recommend the gh CLI for this: gh pr checkout 42 should get you there.

@luckysori
Copy link
Copy Markdown
Collaborator

Yes, if @da-kami has the checkbox at the right end of the PR checked (which it is by default), you can checkout a PR locally and continue developing.

I'd recommend the gh CLI for this: gh pr checkout 42 should get you there.

Thanks for the tip! I'm using gh now.

I've managed to check out the PR and made the changes and I want to push to this PR, but I can't push directly to Daniel's branch. Should I close this PR and push to a different branch in this repo?

@thomaseizinger
Copy link
Copy Markdown
Contributor

Yes, if @da-kami has the checkbox at the right end of the PR checked (which it is by default), you can checkout a PR locally and continue developing.
I'd recommend the gh CLI for this: gh pr checkout 42 should get you there.

Thanks for the tip! I'm using gh now.

I've managed to check out the PR and made the changes and I want to push to this PR, but I can't push directly to Daniel's branch. Should I close this PR and push to a different branch in this repo?

If you can't push directly then @da-kami would need to enable "Allow edits by maintainers" on the bottom right of the toolbar. Alternatively, just open a new PR :)

@luckysori
Copy link
Copy Markdown
Collaborator

Superseded by #45. See: #42 (comment)

@luckysori luckysori closed this Aug 3, 2021
@da-kami
Copy link
Copy Markdown
Member Author

da-kami commented Aug 3, 2021

Yes, if @da-kami has the checkbox at the right end of the PR checked (which it is by default), you can checkout a PR locally and continue developing.
I'd recommend the gh CLI for this: gh pr checkout 42 should get you there.

Thanks for the tip! I'm using gh now.
I've managed to check out the PR and made the changes and I want to push to this PR, but I can't push directly to Daniel's branch. Should I close this PR and push to a different branch in this repo?

If you can't push directly then @da-kami would need to enable "Allow edits by maintainers" on the bottom right of the toolbar. Alternatively, just open a new PR :)

Thanks for the explanation @thomaseizinger - next time I'll make sure to tick it :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants