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

Problem: not possible to cancel SendToEthereum transactions (fixes #389) #532

Merged
Merged
Show file tree
Hide file tree
Changes from 17 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
- [cronos#418](https://github.com/crypto-org-chain/cronos/pull/418) Support logs in evm-hooks and return id for SendToEthereum events
- [cronos#489](https://github.com/crypto-org-chain/cronos/pull/489) Enable jemalloc memory allocator, and update rocksdb src to `v6.29.5`.
- [cronos#511](https://github.com/crypto-org-chain/cronos/pull/511) Replace ibc-hook with ibc middleware, use ibc-go upstream version.
- [cronos#532](https://github.com/crypto-org-chain/cronos/pull/532) Add CancelSendToEthereum support from evm call.

*May 3, 2022*

Expand Down
3 changes: 2 additions & 1 deletion app/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -488,7 +488,8 @@ func New(

app.EvmKeeper.SetHooks(cronoskeeper.NewLogProcessEvmHook(
cronoskeeper.NewSendToAccountHandler(app.BankKeeper, app.CronosKeeper),
cronoskeeper.NewSendToEthereumHandler(gravitySrv, app.CronosKeeper),
cronoskeeper.NewSendToEthereumHandler(gravitySrv, app.BankKeeper, app.CronosKeeper),
cronoskeeper.NewCancelSendToEthereumHandler(gravitySrv, app.CronosKeeper, app.GravityKeeper),
cronoskeeper.NewSendToIbcHandler(app.BankKeeper, app.CronosKeeper),
cronoskeeper.NewSendCroToIbcHandler(app.BankKeeper, app.CronosKeeper),
))
Expand Down
2 changes: 1 addition & 1 deletion contracts/src/ModuleCRC20.sol
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ contract ModuleCRC20 is DSToken {
unsafe_burn(addr, amount);
}

// send to ethereum through gravity bridge
// DEPRECATED: send to ethereum through gravity bridge
function send_to_ethereum(address recipient, uint amount, uint bridge_fee) external {
unsafe_burn(msg.sender, add(amount, bridge_fee));
emit __CronosSendToEthereum(recipient, amount, bridge_fee);
Expand Down
34 changes: 34 additions & 0 deletions contracts/src/ModuleCRC21.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
pragma solidity ^0.6.8;

import "./ModuleCRC20.sol";

contract ModuleCRC21 is ModuleCRC20 {

event __CronosSendToEthereum(address sender, address recipient, uint256 amount, uint256 bridge_fee);
event __CronosCancelSendToEthereum(address sender, uint256 id);

constructor(string memory denom_, uint8 decimals_) ModuleCRC20(denom_, decimals_) public {
decimals = decimals_;
denom = denom_;
}

// make unsafe_burn internal
function unsafe_burn_v2(address addr, uint amount) internal {
// Deduct user's balance without approval
require(balanceOf[addr] >= amount, "ds-token-insufficient-balance");
balanceOf[addr] = sub(balanceOf[addr], amount);
totalSupply = sub(totalSupply, amount);
emit Burn(addr, amount);
}

// send to ethereum through gravity bridge
thomas-nguy marked this conversation as resolved.
Show resolved Hide resolved
function send_to_ethereum_v2(address recipient, uint amount, uint bridge_fee) external {
Copy link
Collaborator Author

@thomas-nguy thomas-nguy Jun 8, 2022

Choose a reason for hiding this comment

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

Without having to migrate all existing contract, it is possible to create a wrapper over the CRC20 to be able to call __CronosSendToEthereum and have a separate supply from the original CRC20

It would be cleaner to remove send_to_ethereum from the CRC20.sol and remove the _v2 here however this might change the bytecode of the original CRC20 standard (even though nobody is using this method now)

Any opinion @tomtau @yihuang

Copy link
Collaborator

Choose a reason for hiding this comment

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

Agree with the standard extension, and deprecate the send_to_ethereum from the original one.
But existing contracts still need to migrate code to support the gravity bridge, right?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

another idea is to have a wrapper over exsting CRC20 contract to be able to add the ability to bridge.
For example having ETH_VVS will be a wrapper of VVS.sol and expose send_to_ethereum and wrapping/unwrapping functions
The 1-1 supply control will be guarantee by the smart contract

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd suggest documenting these architectural decisions: https://github.com/crypto-org-chain/cronos/blob/main/docs/architecture/adr-template.md + getting some feedback from dApp developers on it

Copy link
Collaborator

@yihuang yihuang Jun 8, 2022

Choose a reason for hiding this comment

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

another idea is to have a wrapper over exsting CRC20 contract to be able to add the ability to bridge. For example having ETH_VVS will be a wrapper of VVS.sol and expose send_to_ethereum and wrapping/unwrapping functions The 1-1 supply control will be guarantee by the smart contract

it's still a contract migration, the client need to use a different contract address, but internally it reuse the old contract to avoid state migration, right?
I think dapps could have multiple solutions to do this, sometimes even have the upgradability build-in.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

the client need to use a different contract address, but internally it reuse the old contract to avoid state migration

Why does it need to use old contract? Mapping between gravitydenom <-> contract address will be done on the wrapped contract.
The drawback is that to be able to send to ethereum, one needs to wrap its token first (or unwrap for the opposite direction) but that can be integrated seeminglessly by the Dapp

Copy link
Collaborator

Choose a reason for hiding this comment

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

I see, you mean a 1-to-1 token wrapping contract, I guess for erc20, user will need to do a) approve, b) swap, c) send_to_ethereum, right?

unsafe_burn_v2(msg.sender, add(amount, bridge_fee));
emit __CronosSendToEthereum(msg.sender, recipient, amount, bridge_fee);
}

// cancel a send to ethereum transaction considering if it hasnt been batched yet.
function cancel_send_to_ethereum(uint256 id) external {
emit __CronosCancelSendToEthereum(msg.sender, id);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
pragma solidity ^0.6.6;

contract CronosGravityCancellation {

event __CronosCancelSendToEthereum(address sender, uint256 id);

// Cancel a send to ethereum transaction considering if it hasnt been batched yet.
function cancelTransaction(uint256 id) public {
emit __CronosCancelSendToEthereum(msg.sender, id);
}
}
136 changes: 117 additions & 19 deletions integration_tests/test_gravity.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@
Account.enable_unaudited_hdwallet_features()


def cronos_crc20_abi():
path = CONTRACTS["ModuleCRC20"]
def cronos_crc21_abi():
path = CONTRACTS["ModuleCRC21"]
return json.load(path.open())["abi"]


Expand Down Expand Up @@ -209,21 +209,21 @@ def test_gravity_transfer(gravity):

denom = f"gravity{erc20.address}"

crc20_contract = None
crc21_contract = None

def check_auto_deployment():
"check crc20 contract auto deployed, and the crc20 balance"
nonlocal crc20_contract
nonlocal crc21_contract
try:
rsp = cli.query_contract_by_denom(denom)
except AssertionError:
# not deployed yet
return False
assert len(rsp["auto_contract"]) > 0
crc20_contract = cronos_w3.eth.contract(
address=rsp["auto_contract"], abi=cronos_crc20_abi()
crc21_contract = cronos_w3.eth.contract(
address=rsp["auto_contract"], abi=cronos_crc21_abi()
)
return crc20_contract.caller.balanceOf(recipient) == amount
return crc21_contract.caller.balanceOf(recipient) == amount

def check_gravity_native_tokens():
"check the balance of gravity native token"
Expand All @@ -244,7 +244,7 @@ def get_id_from_receipt(receipt):
wait_for_fn("send-to-crc20", check_auto_deployment)

# send it back to erc20
tx = crc20_contract.functions.send_to_ethereum(
tx = crc21_contract.functions.send_to_ethereum_v2(
ADDRS["validator"], amount, 0
).buildTransaction({"from": ADDRS["community"]})
txreceipt = send_transaction(cronos_w3, tx, KEYS["community"])
Expand Down Expand Up @@ -291,19 +291,19 @@ def test_gov_token_mapping(gravity):
CONTRACTS["TestERC20A"],
)
print("erc20 contract", erc20.address)
crc20 = deploy_contract(
crc21 = deploy_contract(
cronos_w3,
CONTRACTS["TestERC20Utility"],
)
print("crc20 contract", crc20.address)
print("crc21 contract", crc21.address)
denom = f"gravity{erc20.address}"

print("check the contract mapping not exists yet")
with pytest.raises(AssertionError):
cli.query_contract_by_denom(denom)

rsp = cli.gov_propose_token_mapping_change(
denom, crc20.address, from_="community", deposit="1basetcro"
denom, crc21.address, from_="community", deposit="1basetcro"
)
assert rsp["code"] == 0, rsp["raw_log"]

Expand Down Expand Up @@ -332,7 +332,7 @@ def test_gov_token_mapping(gravity):
print("check the contract mapping exists now")
rsp = cli.query_contract_by_denom(denom)
print("contract", rsp)
assert rsp["contract"] == crc20.address
assert rsp["contract"] == crc21.address

print("try to send token from ethereum to cronos")
txreceipt = send_to_cosmos(
Expand All @@ -341,7 +341,7 @@ def test_gov_token_mapping(gravity):
assert txreceipt.status == 1

def check():
balance = crc20.caller.balanceOf(ADDRS["community"])
balance = crc21.caller.balanceOf(ADDRS["community"])
print("crc20 balance", balance)
return balance == 10

Expand All @@ -366,28 +366,28 @@ def test_direct_token_mapping(gravity):
CONTRACTS["TestERC20A"],
)
print("erc20 contract", erc20.address)
crc20 = deploy_contract(
crc21 = deploy_contract(
cronos_w3,
CONTRACTS["TestERC20Utility"],
)
print("crc20 contract", crc20.address)
print("crc21 contract", crc21.address)
denom = f"gravity{erc20.address}"

print("check the contract mapping not exists yet")
with pytest.raises(AssertionError):
cli.query_contract_by_denom(denom)

rsp = cli.update_token_mapping(denom, crc20.address, from_="community")
rsp = cli.update_token_mapping(denom, crc21.address, from_="community")
assert rsp["code"] != 0, "should not have the permission"

rsp = cli.update_token_mapping(denom, crc20.address, from_="validator")
rsp = cli.update_token_mapping(denom, crc21.address, from_="validator")
assert rsp["code"] == 0, rsp["raw_log"]
wait_for_new_blocks(cli, 1)

print("check the contract mapping exists now")
rsp = cli.query_contract_by_denom(denom)
print("contract", rsp)
assert rsp["contract"] == crc20.address
assert rsp["contract"] == crc21.address

print("try to send token from ethereum to cronos")
txreceipt = send_to_cosmos(
Expand All @@ -396,8 +396,106 @@ def test_direct_token_mapping(gravity):
assert txreceipt.status == 1

def check():
balance = crc20.caller.balanceOf(ADDRS["community"])
balance = crc21.caller.balanceOf(ADDRS["community"])
print("crc20 balance", balance)
return balance == 10

wait_for_fn("check balance on cronos", check)


def test_gravity_cancel_transfer(gravity):
if gravity.cronos.enable_auto_deployment:
geth = gravity.geth
cli = gravity.cronos.cosmos_cli()
cronos_w3 = gravity.cronos.w3

# deploy test erc20 contract
erc20 = deploy_contract(
geth,
CONTRACTS["TestERC20A"],
)

# deploy gravity cancellation contract
cancel_contract = deploy_contract(
cronos_w3,
CONTRACTS["CronosGravityCancellation"],
)

balance = erc20.caller.balanceOf(ADDRS["validator"])
assert balance == 100000000000000000000000000
amount = 1000

print("send to cronos crc21")
recipient = HexBytes(ADDRS["community"])
send_to_cosmos(gravity.contract, erc20, recipient, amount, KEYS["validator"])
assert erc20.caller.balanceOf(ADDRS["validator"]) == balance - amount

denom = f"gravity{erc20.address}"

crc21_contract = None

def check_auto_deployment():
"check crc21 contract auto deployed, and the crc21 balance"
nonlocal crc21_contract
try:
rsp = cli.query_contract_by_denom(denom)
except AssertionError:
# not deployed yet
return False
assert len(rsp["auto_contract"]) > 0
crc21_contract = cronos_w3.eth.contract(
address=rsp["auto_contract"], abi=cronos_crc21_abi()
)
return crc21_contract.caller.balanceOf(recipient) == amount

def get_id_from_receipt(receipt):
"check the id after sendToEthereum call"
for _, log in enumerate(receipt.logs):
if log.topics[0] == HexBytes(
abi.event_signature_to_log_topic(
"__CronosSendToEthereumResponse(uint256)"
)
):
return log.data
return "0x0000000000000000000000000000000000000000000000000000000000000000"

wait_for_fn("send-to-crc20", check_auto_deployment)

def check_fund():
v = crc21_contract.caller.balanceOf(ADDRS["community"])
return v == amount

wait_for_fn("send-to-ethereum", check_fund)

# send it back to erc20
tx = crc21_contract.functions.send_to_ethereum_v2(
ADDRS["validator"], amount, 0
).buildTransaction({"from": ADDRS["community"]})
txreceipt = send_transaction(cronos_w3, tx, KEYS["community"])
# CRC20 emit 3 logs for send_to_ethereum:
# burn
# __CronosSendToEthereum
# __CronosSendToEthereumResponse
assert len(txreceipt.logs) == 3
tx_id = get_id_from_receipt(txreceipt)
assert txreceipt.status == 1, "should success"

def check_deduction():
v = crc21_contract.caller.balanceOf(ADDRS["community"])
return v == 0

wait_for_fn("check deduction", check_deduction)
yihuang marked this conversation as resolved.
Show resolved Hide resolved

# Cancel the send_to_ethereum
canceltx = cancel_contract.functions.cancelTransaction(
int(tx_id, base=16)
).buildTransaction({"from": ADDRS["community"]})
canceltxreceipt = send_transaction(cronos_w3, canceltx, KEYS["community"])
print("canceltxreceipt", canceltxreceipt)
assert canceltxreceipt.status == 1, "should success"

def check_refund():
v = crc21_contract.caller.balanceOf(ADDRS["community"])
return v == amount

wait_for_fn("cancel-send-to-ethereum", check_refund)
3 changes: 3 additions & 0 deletions integration_tests/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
"TestERC20Utility": "TestERC20Utility.sol",
"TestMessageCall": "TestMessageCall.sol",
"CroBridge": "CroBridge.sol",
"CronosGravityCancellation": "CronosGravityCancellation.sol",
}


Expand All @@ -55,6 +56,8 @@ def contract_path(name, filename):
CONTRACTS = {
"ModuleCRC20": Path(__file__).parent.parent
/ "x/cronos/types/contracts/ModuleCRC20.json",
"ModuleCRC21": Path(__file__).parent.parent
/ "x/cronos/types/contracts/ModuleCRC21.json",
**{
name: contract_path(name, filename) for name, filename in TEST_CONTRACTS.items()
},
Expand Down
1 change: 1 addition & 0 deletions scripts/cronos-experimental-devnet.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ cronos_777-1:
block:
max_bytes: "1048576"
max_gas: "81500000"
time_iota_ms: "2000"
thomas-nguy marked this conversation as resolved.
Show resolved Hide resolved
app_state:
evm:
params:
Expand Down
4 changes: 4 additions & 0 deletions scripts/gen-cronos-contracts
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,7 @@ cat contracts/out/dapp.sol.json | \
jq '.contracts."src/ModuleCRC20.sol".ModuleCRC20' | \
jq '{abi, bin: .evm.bytecode.object}' \
> x/cronos/types/contracts/ModuleCRC20.json
cat contracts/out/dapp.sol.json | \
jq '.contracts."src/ModuleCRC21.sol".ModuleCRC21' | \
jq '{abi, bin: .evm.bytecode.object}' \
> x/cronos/types/contracts/ModuleCRC21.json
10 changes: 5 additions & 5 deletions x/cronos/keeper/evm.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,13 +54,13 @@ func (k Keeper) CallModuleCRC20(ctx sdk.Context, contract common.Address, method
return res.Ret, nil
}

// DeployModuleCRC20 deploy an embed erc20 contract
func (k Keeper) DeployModuleCRC20(ctx sdk.Context, denom string) (common.Address, error) {
ctor, err := types.ModuleCRC20Contract.ABI.Pack("", denom, uint8(0))
// DeployModuleCRC21 deploy an embed erc20 contract
yihuang marked this conversation as resolved.
Show resolved Hide resolved
func (k Keeper) DeployModuleCRC21(ctx sdk.Context, denom string) (common.Address, error) {
ctor, err := types.ModuleCRC21Contract.ABI.Pack("", denom, uint8(0))
if err != nil {
return common.Address{}, err
}
data := types.ModuleCRC20Contract.Bin
data := types.ModuleCRC21Contract.Bin
data = append(data, ctor...)

msg, res, err := k.CallEVM(ctx, nil, data, big.NewInt(0))
Expand All @@ -87,7 +87,7 @@ func (k Keeper) ConvertCoinFromNativeToCRC20(ctx sdk.Context, sender common.Addr
if !autoDeploy {
return fmt.Errorf("no contract found for the denom %s", coin.Denom)
}
contract, err = k.DeployModuleCRC20(ctx, coin.Denom)
contract, err = k.DeployModuleCRC21(ctx, coin.Denom)
if err != nil {
return err
}
Expand Down
Loading