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.

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

Primata/405 move get bridge transfer details #434

Merged
merged 21 commits into from
Aug 26, 2024

Conversation

Primata
Copy link
Contributor

@Primata Primata commented Aug 22, 2024

Summary

Handles move get bridge details by calling view function bridge_transfers(). It takes a bridge_transfer_id variable.

We might want to pay attention since it copy ability had to be added to struct BridgeTransfers

Because bridge_transfer requires a single SmartTable, counterparty had to be refactored.

Changelog

fix init_modules functions
fix tests
complete, refund or cancel bridge transfers do not delete transfers anymore
refactor
bridge_transfers view function to retrieve bridge_transfer details by specifying a transfer id

Testing

Tests are fine.

Outstanding issues

We might want to look to test thoroughly Counterparty modules as they are not completely covered.

@0xmovses 0xmovses changed the base branch from main to eng-546-atomic-bridge August 22, 2024 19:20
Copy link
Contributor

@0xmovses 0xmovses left a comment

Choose a reason for hiding this comment

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

One question on building the zeroed BridgeTransfer.

@andyjsbell
Copy link
Contributor

Do we want to add copy to this type? Probably a tuple would be a better design here.

@Primata
Copy link
Contributor Author

Primata commented Aug 23, 2024

@andyjsbell do you mean returning a tuple? Certainly, just wondering how to do that exactly. Any suggestions on how to remove copy are welcome.

@andygolay
Copy link
Contributor

@andyjsbell do you mean returning a tuple? Certainly, just wondering how to do that exactly. Any suggestions on how to remove copy are welcome.

I updated it to return a tuple (see this commit: d90348c). This allows removing the copy and drop ability, which don't really seem like abilities we want for BridgeTransfer.

(I changed the name of bridge_transfers to get_bridge_transfer_from_id to make it describe what the function does.

Copy link
Contributor

@andygolay andygolay left a comment

Choose a reason for hiding this comment

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

LGTM, I made some commits to address review comments by @0xmovses and @andyjsbell and to make CI pass.

@andygolay andygolay changed the title Primata/405 move get bridge details Primata/405 move get bridge transfer details Aug 25, 2024
@andygolay andygolay merged commit 68532c3 into eng-546-atomic-bridge Aug 26, 2024
34 checks passed
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.

4 participants