-
Notifications
You must be signed in to change notification settings - Fork 20
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
Atomic Bridge: Add get_bridge_transfer_details
view function and fix duplicate create_time_lock
calls
#85
Atomic Bridge: Add get_bridge_transfer_details
view function and fix duplicate create_time_lock
calls
#85
Conversation
|
||
*details_ref | ||
} | ||
|
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.
Would this be clearer? Would need documentation around this I think.
#[view]
public fun get_bridge_transfer_details_initiator(
bridge_transfer_id: vector<u8>
): BridgeTransferDetails<address, EthereumAddress> acquires SmartTableWrapper {
get_bridge_transfer_details(bridge_transfer_id)
}
#[view]
public fun get_bridge_transfer_details_counterparty(
bridge_transfer_id: vector<u8>
): BridgeTransferDetails<EthereumAddress, address> acquires SmartTableWrapper {
get_bridge_transfer_details(bridge_transfer_id)
}
fun get_bridge_transfer_details<Initiator: store + copy, Recipient: store + copy>(bridge_transfer_id: vector<u8>
): BridgeTransferDetails<Initiator, Recipient> acquires SmartTableWrapper {
let table = borrow_global<SmartTableWrapper<vector<u8>, BridgeTransferDetails<Initiator, Recipient>>>(@aptos_framework);
let details_ref = smart_table::borrow(
&table.inner,
bridge_transfer_id
);
*details_ref
}
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.
Updated, yep clearer, thanks.
Added doc comments to the two new view functions.
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.
Look good, Danke.
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.
Thanks @andygolay
Description
See #83 , opened this due to issues working with the branch after merge to
movement
was reverted.get_bridge_transfers_initiator
andget_bridge_transfers_counterparty
view functions intoatomic_bridge_store
module with corresponding Move unit tests. All tests pass withmovement move test
create_time_lock
was called twice, resulting in time locks farther into the future than expected.Type of Change
Which Components or Systems Does This Change Impact?
How Has This Been Tested?
In
aptos-move/framework/aptos-framework/sources
runmovement move test
oraptos move test
.Key Areas to Review
I think this works better as two separate functions because of the way it aligns with the
BridgeContract
trait in our Rust code. However, it could be possible to combine them into one function if someone wants to do that. For now this seems like a good solution to being able to fetch details without relying on event monitoring.