Skip to content

contracts-bedrock: depositTransaction warning#2678

Merged
mergify[bot] merged 4 commits intodevelopfrom
m/tob-7-underspecified-deposits
Jun 3, 2022
Merged

contracts-bedrock: depositTransaction warning#2678
mergify[bot] merged 4 commits intodevelopfrom
m/tob-7-underspecified-deposits

Conversation

@maurelian
Copy link
Contributor

@maurelian maurelian commented Jun 3, 2022

Addresses TOB-OPT-7:

When a smart contract submits a deposit transaction, the code will transform the contract address to an aliased address by adding a fixed offset. Due to the lack of specification and guidance regarding how smart contracts should manage funds within the system, a naive smart contract that interacts with the DepositFeed could lock funds in the system that may not be retrievable later.

The recommendation was:

Provide guidance within the documentation to ensure that smart contract developers are aware of this edge case.

@changeset-bot
Copy link

changeset-bot bot commented Jun 3, 2022 •
edited
Loading

⚠️ No Changeset found

Latest commit: d37bc73

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@maurelian maurelian requested review from smartcontracts and tynes June 3, 2022 18:35
@mergify mergify bot requested review from mslipper and tuxcanfly June 3, 2022 18:35
@maurelian maurelian force-pushed the m/tob-7-underspecified-deposits branch from 2c3add6 to 44ac024 Compare June 3, 2022 18:58
@smartcontracts
Copy link
Contributor

smartcontracts commented Jun 3, 2022

Needs lint fixes

@maurelian maurelian force-pushed the m/tob-7-underspecified-deposits branch from 44ac024 to 7548d9e Compare June 3, 2022 19:23
@maurelian
contracts-bedrock: depositTransaction warning
58326fe 
Addresses TOB-OPT-7:
> When a smart contract submits a deposit transaction, the code will transform the contract address to an aliased address by adding a fixed offset. Due to the lack of specification and guidance regarding how smart contracts should manage funds within the system, a naive smart contract that interacts with the DepositFeed could lock funds in the system that may not be retrievable later.

The recommendation was:
> Provide guidance within the documentation to ensure that smart contract developers are aware of this edge case.
@maurelian maurelian force-pushed the m/tob-7-underspecified-deposits branch from 7548d9e to 58326fe Compare June 3, 2022 19:41
@mergify
Copy link
Contributor

mergify bot commented Jun 3, 2022

This PR has been added to the merge queue, and will be merged soon.

@mergify
Copy link
Contributor

mergify bot commented Jun 3, 2022

This PR is next in line to be merged, and will be merged as soon as checks pass.

@mergify
Copy link
Contributor

mergify bot commented Jun 3, 2022

Merge failed. Please see automated check logs for more details.

@mergify
Copy link
Contributor

mergify bot commented Jun 3, 2022

Hey @maurelian! This PR has merge conflicts. Please fix them before continuing review.

@mergify mergify bot added the conflict label Jun 3, 2022
@mergify mergify bot removed the conflict label Jun 3, 2022
@mergify mergify bot merged commit e031de8 into develop Jun 3, 2022
@mergify mergify bot deleted the m/tob-7-underspecified-deposits branch June 3, 2022 22:40
@mergify
Copy link
Contributor

mergify bot commented Jun 3, 2022

This PR is next in line to be merged, and will be merged as soon as checks pass.

@mslipper mslipper mentioned this pull request Jun 6, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mslipper mslipper mslipper approved these changes

@tynes tynes tynes approved these changes

@smartcontracts smartcontracts smartcontracts approved these changes

@tuxcanfly tuxcanfly Awaiting requested review from tuxcanfly

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@maurelian @smartcontracts @mslipper @tynes