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

Sponsored Atomic Bridge #104

Open
wants to merge 1 commit into
base: movement
Choose a base branch
from

Conversation

l-monninger
Copy link
Collaborator

@l-monninger l-monninger commented Nov 22, 2024

Description

Sponsors bridge transfers somewhat safely:

  • Uses the balance of the framework minter to control max exploit.
  • Requires a minimum transfer amount--ensuring attempt to exploit is costly for a small gain.
  • Deducts sponsorship from transfer amount. Thus, insofar as bridge operation is trusted, the exploit cannot exceed the amount. And, in the case where refunds require Movement Labs approval, an exploit can always be averted.

To test:

movement move test -f sponsor

To be safer, include the sponsorship fee from movementlabsxyz/movement#896. As long as the fee is greater than or equal to sponsored amount, this is safe.

Type of Change

  • New feature
  • Bug fix
  • Breaking change
  • Performance improvement
  • Refactoring
  • Dependency update
  • Documentation update
  • Tests

Which Components or Systems Does This Change Impact?

  • Validator Node
  • Full Node (API, Indexer, etc.)
  • Move/Aptos Virtual Machine
  • Aptos Framework
  • Aptos CLI/SDK
  • Developer Infrastructure
  • Other (specify)

How Has This Been Tested?

Key Areas to Review

Checklist

  • I have read and followed the CONTRIBUTING doc
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I identified and added all stakeholders and component owners affected by this change as reviewers
  • I tested both happy and unhappy path of the functionality
  • I have made corresponding changes to the documentation

@l-monninger l-monninger changed the title fix: unrevert. Sponsored Atomic Bridge Nov 22, 2024
!account::exists_at(recipient) || coin::balance<AptosCoin>(recipient) == 0
}

/// Sponsors a recipient by transferring enought Aptos Coint to make the claim to them

Choose a reason for hiding this comment

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

Suggested change
/// Sponsors a recipient by transferring enought Aptos Coint to make the claim to them
/// Sponsors a recipient by transferring enought Aptos Coin to make the claim to them

Copy link

@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; logic makes sense and tests pass locally.

@andygolay andygolay linked an issue Nov 22, 2024 that may be closed by this pull request

#[test(aptos_framework = @aptos_framework, third_party = @0xdead)]
/// Test ineligible sponsor
fun test_lock_for_ineligible_sponsor(aptos_framework: &signer, third_party: &signer) acquires BridgeCounterpartyEvents {

Choose a reason for hiding this comment

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

Suggested change
fun test_lock_for_ineligible_sponsor(aptos_framework: &signer, third_party: &signer) acquires BridgeCounterpartyEvents {
fun test_lock_for_ineligible_sponsorship(aptos_framework: &signer, third_party: &signer) acquires BridgeCounterpartyEvents {

This isn't a test of whether a specific sponsor is eligible, but whether a transfer is eligible for sponsorship.

amount
}

/// Sponsors a recipient by transferring enough Aptos Coin to make the claim to them if they are eligible
Copy link

@andygolay andygolay Nov 22, 2024

Choose a reason for hiding this comment

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

Suggested change
/// Sponsors a recipient by transferring enough Aptos Coin to make the claim to them if they are eligible
/// Sponsors a recipient by transferring enough Aptos Coin for them to claim if the transfer is eligible


#[test(aptos_framework = @aptos_framework, third_party = @0xdead)]
/// Test eligible sponsor
fun test_lock_for_eligible_sponsor(aptos_framework: &signer, third_party: &signer) acquires BridgeCounterpartyEvents {

Choose a reason for hiding this comment

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

Suggested change
fun test_lock_for_eligible_sponsor(aptos_framework: &signer, third_party: &signer) acquires BridgeCounterpartyEvents {
fun test_lock_for_eligible_sponsorship(aptos_framework: &signer, third_party: &signer) acquires BridgeCounterpartyEvents {

This isn't a test of whether a specific sponsor is eligible, but whether a transfer is eligible for sponsorship.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Agreed

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.

Bridge Sponsored Gas (Initial)
2 participants