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.

Sign up for GitHub

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

Jump to bottom

feat(evm-funtoken-precompile): Implement "balance", "bankBalance", and "whoAmI" methods #2107

Merged
merged 8 commits into from
Nov 8, 2024
Merged

feat(evm-funtoken-precompile): Implement "balance", "bankBalance", and "whoAmI" methods #2107

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
3aef202
refactor(evm)!: rename IFunToken.bankSend -> IFunToken.sendToBank
Unique-Divine Nov 6, 2024
d349166
impl IFuntoken.balance, IFunToken.bankBalance
Unique-Divine Nov 6, 2024
5ed7cf4
chore: changelog
Unique-Divine Nov 6, 2024
e748b3c
impl whoAmI query and do cleanup
Unique-Divine Nov 8, 2024
4360ae2
changelog + fix inconsistent test
Unique-Divine Nov 8, 2024
08c0b20
soften assertion on non-deterministic condition
Unique-Divine Nov 8, 2024
06f46f3
type error
Unique-Divine Nov 8, 2024
9d15c07
docs
Unique-Divine Nov 8, 2024
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
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,8 @@ tests for race conditions within funtoken precompile
- [#2100](https://github.com/NibiruChain/nibiru/pull/2100) - refactor: cleanup statedb and precompile sections
- [#2101](https://github.com/NibiruChain/nibiru/pull/2101) - fix(evm): tx receipt proper marshalling
- [#2105](https://github.com/NibiruChain/nibiru/pull/2105) - test(evm): precompile call with revert
- [#2107](https://github.com/NibiruChain/nibiru/pull/2107) -
feat(evm-funtoken-precompile): Implement methods: balance, bankBalance, whoAmI

#### Nibiru EVM | Before Audit 1 - 2024-10-18

Expand Down
3 changes: 2 additions & 1 deletion eth/rpc/backend/account_info.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,8 @@ func (b *Backend) GetProof(
}
ctx := rpc.NewContextWithHeight(height)

// if the height is equal to zero, meaning the query condition of the block is either "pending" or "latest"
// if the height is equal to zero, meaning the query condition of the block
// is either "pending" or "latest"
if height == 0 {
bn, err := b.BlockNumber()
if err != nil {
Expand Down
15 changes: 9 additions & 6 deletions eth/rpc/rpcapi/eth_api_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,11 @@ func (s *NodeSuite) Test_BlockNumber() {

ethBlockNumber, err := s.ethClient.BlockNumber(context.Background())
s.NoError(err)
s.Equal(networkBlockNumber, int64(ethBlockNumber))
// It might be off by 1 block in either direction.
blockDiff := networkBlockNumber - int64(ethBlockNumber)
s.Truef(blockDiff <= 2, "networkBlockNumber %d, ethBlockNumber %d",
networkBlockNumber, ethBlockNumber,
)
}

// Test_BlockByNumber EVM method: eth_getBlockByNumber
Expand Down Expand Up @@ -366,11 +370,10 @@ func (s *NodeSuite) Test_SmartContract() {
txHash, err := s.ethAPI.SendRawTransaction(txBz)
s.Require().NoError(err)

s.T().Log("Assert: tx IS pending just after execution")
pendingTxs, err := s.ethAPI.GetPendingTransactions()
s.NoError(err)
s.Require().Len(pendingTxs, 1)
_ = s.network.WaitForNextBlock()
s.T().Log("Wait a few blocks so the tx won't be pending")
for i := 0; i < 5; i++ {
_ = s.network.WaitForNextBlock()
}

s.T().Log("Assert: tx NOT pending")
{
Expand Down
137 changes: 136 additions & 1 deletion x/evm/embeds/artifacts/contracts/IFunToken.sol/IFunToken.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,110 @@
"contractName": "IFunToken",
"sourceName": "contracts/IFunToken.sol",
"abi": [
{
"inputs": [
{
"internalType": "address",
"name": "who",
"type": "address"
},
{
"internalType": "address",
"name": "funtoken",
"type": "address"
}
],
"name": "balance",
"outputs": [
{
"internalType": "uint256",
"name": "erc20Balance",
"type": "uint256"
},
{
"internalType": "uint256",
"name": "bankBalance",
"type": "uint256"
},
{
"components": [
{
"internalType": "address",
"name": "erc20",
"type": "address"
},
{
"internalType": "string",
"name": "bankDenom",
"type": "string"
}
],
"internalType": "struct IFunToken.FunToken",
"name": "token",
"type": "tuple"
},
{
"components": [
{
"internalType": "address",
"name": "ethAddr",
"type": "address"
},
{
"internalType": "string",
"name": "bech32Addr",
"type": "string"
}
],
"internalType": "struct IFunToken.NibiruAccount",
"name": "whoAddrs",
"type": "tuple"
}
],
"stateMutability": "nonpayable",
"type": "function"
},
{
"inputs": [
{
"internalType": "address",
"name": "who",
"type": "address"
},
{
"internalType": "string",
"name": "bankDenom",
"type": "string"
}
],
"name": "bankBalance",
"outputs": [
{
"internalType": "uint256",
"name": "bankBalance",
"type": "uint256"
},
{
"components": [
{
"internalType": "address",
"name": "ethAddr",
"type": "address"
},
{
"internalType": "string",
"name": "bech32Addr",
"type": "string"
}
],
"internalType": "struct IFunToken.NibiruAccount",
"name": "whoAddrs",
"type": "tuple"
}
],
"stateMutability": "nonpayable",
"type": "function"
},
Comment on lines +69 to +109
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider changing state mutability to view.

The bankBalance function is well-designed for querying bank balances with proper denomination support. However, since this is a read-only operation, it should be marked as view instead of nonpayable to save gas and better reflect its behavior.

{
"inputs": [
{
Expand All @@ -21,7 +125,7 @@
"type": "string"
}
],
"name": "bankSend",
"name": "sendToBank",
"outputs": [
{
"internalType": "uint256",
Expand All @@ -31,6 +135,37 @@
],
"stateMutability": "nonpayable",
"type": "function"
},
{
"inputs": [
{
"internalType": "string",
"name": "who",
"type": "string"
}
],
"name": "whoAmI",
"outputs": [
{
"components": [
{
"internalType": "address",
"name": "ethAddr",
"type": "address"
},
{
"internalType": "string",
"name": "bech32Addr",
"type": "string"
}
],
"internalType": "struct IFunToken.NibiruAccount",
"name": "whoAddrs",
"type": "tuple"
}
],
"stateMutability": "nonpayable",
"type": "function"
}
],
"bytecode": "0x",
Expand Down
4 changes: 2 additions & 2 deletions ...ontracts/TestERC20TransferThenPrecompileSend.sol/TestERC20TransferThenPrecompileSend.json
View file
Open in desktop

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions ...rtifacts/contracts/TestFunTokenPrecompileLocalGas.sol/TestFunTokenPrecompileLocalGas.json
View file
Open in desktop

Large diffs are not rendered by default.

22 changes: 20 additions & 2 deletions ...acts/contracts/TestNativeSendThenPrecompileSend.sol/TestNativeSendThenPrecompileSend.json
View file
Open in desktop

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions ...ds/artifacts/contracts/TestPrecompileSelfCallRevert.sol/TestPrecompileSelfCallRevert.json
View file
Open in desktop

Large diffs are not rendered by default.

47 changes: 38 additions & 9 deletions x/evm/embeds/contracts/IFunToken.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,24 +1,53 @@
// SPDX-License-Identifier: MIT
pragma solidity >=0.8.19;

/// @dev Implements the "bankSend" functionality for sending ERC20 tokens as bank
/// coins to a Nibiru bech32 address using the "FunToken" mapping between the
/// ERC20 and bank.
address constant FUNTOKEN_PRECOMPILE_ADDRESS = 0x0000000000000000000000000000000000000800;
IFunToken constant FUNTOKEN_PRECOMPILE = IFunToken(FUNTOKEN_PRECOMPILE_ADDRESS);

/// @dev Implements the functionality for sending ERC20 tokens and bank
/// coins to various Nibiru accounts using either the Nibiru Bech32 address
/// using the "FunToken" mapping between the ERC20 and bank.
interface IFunToken {
/// @dev bankSend sends ERC20 tokens as coins to a Nibiru base account
/// @dev sendToBank sends ERC20 tokens as coins to a Nibiru base account
/// @param erc20 - the address of the ERC20 token contract
/// @param amount - the amount of tokens to send
/// @param to - the receiving Nibiru base account address as a string
/// @return sentAmount - amount of tokens received by the recipient. This may
/// not be equal to `amount` if the corresponding ERC20 contract has a fee or
/// deduction on transfer.
function bankSend(
function sendToBank(
address erc20,
uint256 amount,
string memory to
string calldata to
) external returns (uint256 sentAmount);
}

address constant FUNTOKEN_PRECOMPILE_ADDRESS = 0x0000000000000000000000000000000000000800;
struct NibiruAccount {
address ethAddr;
string bech32Addr;
}
struct FunToken {
address erc20;
string bankDenom;
}

IFunToken constant FUNTOKEN_PRECOMPILE = IFunToken(FUNTOKEN_PRECOMPILE_ADDRESS);
function balance(
address who,
address funtoken
)
external
returns (
uint256 erc20Balance,
uint256 bankBalance,
FunToken memory token,
NibiruAccount memory whoAddrs
);

function bankBalance(
address who,
string calldata bankDenom
) external returns (uint256 bankBalance, NibiruAccount memory whoAddrs);

function whoAmI(
string calldata who
) external returns (NibiruAccount memory whoAddrs);
}
4 changes: 2 additions & 2 deletions x/evm/embeds/contracts/TestERC20TransferThenPrecompileSend.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ contract TestERC20TransferThenPrecompileSend {
"ERC-20 transfer failed"
);

uint256 sentAmount = FUNTOKEN_PRECOMPILE.bankSend(
uint256 sentAmount = FUNTOKEN_PRECOMPILE.sendToBank(
erc20,
precompileAmount,
precompileRecipient
Expand All @@ -34,7 +34,7 @@ contract TestERC20TransferThenPrecompileSend {
require(
sentAmount == precompileAmount,
string.concat(
"IFunToken.bankSend succeeded but transferred the wrong amount",
"IFunToken.sendToBank succeeded but transferred the wrong amount",
"sentAmount ",
Strings.toString(sentAmount),
"expected ",
Expand Down
14 changes: 7 additions & 7 deletions x/evm/embeds/contracts/TestFunTokenPrecompileLocalGas.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,21 +11,21 @@ contract TestFunTokenPrecompileLocalGas {
erc20 = erc20_;
}

// Calls bankSend of the FunToken Precompile with the default gas.
// Calls sendToBank of the FunToken Precompile with the default gas.
// Internal call could use all the gas for the parent call.
function callBankSend(
uint256 amount,
string memory bech32Recipient
) public {
uint256 sentAmount = FUNTOKEN_PRECOMPILE.bankSend(
uint256 sentAmount = FUNTOKEN_PRECOMPILE.sendToBank(
erc20,
amount,
amount,
bech32Recipient
);
require(
sentAmount == amount,
string.concat(
"IFunToken.bankSend succeeded but transferred the wrong amount",
"IFunToken.sendToBank succeeded but transferred the wrong amount",
"sentAmount ",
Strings.toString(sentAmount),
"expected ",
Expand All @@ -34,22 +34,22 @@ contract TestFunTokenPrecompileLocalGas {
);
}

// Calls bankSend of the FunToken Precompile with the gas amount set in parameter.
// Calls sendToBank of the FunToken Precompile with the gas amount set in parameter.
// Internal call should fail if the gas provided is insufficient.
function callBankSendLocalGas(
uint256 amount,
string memory bech32Recipient,
uint256 customGas
) public {
uint256 sentAmount = FUNTOKEN_PRECOMPILE.bankSend{gas: customGas}(
uint256 sentAmount = FUNTOKEN_PRECOMPILE.sendToBank{gas: customGas}(
erc20,
amount,
bech32Recipient
);
require(
sentAmount == amount,
string.concat(
"IFunToken.bankSend succeeded but transferred the wrong amount",
"IFunToken.sendToBank succeeded but transferred the wrong amount",
"sentAmount ",
Strings.toString(sentAmount),
"expected ",
Expand Down
25 changes: 23 additions & 2 deletions x/evm/embeds/contracts/TestNativeSendThenPrecompileSend.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,36 @@ contract TestNativeSendThenPrecompileSend {
bool isSent = nativeRecipient.send(nativeAmount);
require(isSent, "Failed to send native token");

uint256 sentAmount = FUNTOKEN_PRECOMPILE.bankSend(
uint256 sentAmount = FUNTOKEN_PRECOMPILE.sendToBank(
erc20,
precompileAmount,
precompileRecipient
);
require(
sentAmount == precompileAmount,
string.concat(
"IFunToken.bankSend succeeded but transferred the wrong amount",
"IFunToken.sendToBank succeeded but transferred the wrong amount",
"sentAmount ",
Strings.toString(sentAmount),
"expected ",
Strings.toString(precompileAmount)
)
);
}

function justPrecompileSend(
string memory precompileRecipient,
uint256 precompileAmount
) public {
uint256 sentAmount = FUNTOKEN_PRECOMPILE.sendToBank(
erc20,
precompileAmount,
precompileRecipient
);
require(
sentAmount == precompileAmount,
string.concat(
"IFunToken.sendToBank succeeded but transferred the wrong amount",
"sentAmount ",
Strings.toString(sentAmount),
"expected ",
Expand Down
8 changes: 4 additions & 4 deletions x/evm/embeds/contracts/TestPrecompileSelfCallRevert.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,16 +40,16 @@ contract TestPrecompileSelfCallRevert {
) external {
require(nativeRecipient.send(nativeAmount), "ETH transfer failed"); // wei

uint256 sentAmount = FUNTOKEN_PRECOMPILE.bankSend(
uint256 sentAmount = FUNTOKEN_PRECOMPILE.sendToBank(
erc20,
precompileAmount, // micro-WNIBI
precompileRecipient
precompileAmount, // micro-WNIBI
precompileRecipient
Comment on lines +43 to +46
Copy link
Contributor

Choose a reason for hiding this comment

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

💡 Codebase verification

Based on the search results, I can generate the final response:

Method renaming has been consistently applied across the codebase

The only remaining instance of bankSend is in documentation (x/devgas/v1/README.md), while all implementation files correctly use sendToBank. The documentation reference should be updated for consistency.

  • Update needed in: x/devgas/v1/README.md
🔗 Analysis chain

LGTM! Verify consistent method renaming across the codebase.

The method renaming from bankSend to sendToBank is more intuitive and matches the PR objectives.

Let's verify the consistent renaming across the codebase:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for any remaining instances of 'bankSend' that might have been missed
# and confirm all occurrences have been updated to 'sendToBank'

echo "Checking for any remaining 'bankSend' instances:"
rg "bankSend"

echo -e "\nVerifying 'sendToBank' implementations:"
rg "sendToBank"

Length of output: 4729

);

require(
sentAmount == precompileAmount,
string.concat(
"IFunToken.bankSend succeeded but transferred the wrong amount",
"IFunToken.sendToBank succeeded but transferred the wrong amount",
"sentAmount ",
Strings.toString(nativeAmount),
"expected ",
Expand Down
Loading
Loading