Skip to content

feat(cli): add simulate command for testing RBTC and ERC20 transfers …#288

Merged
ezequiel-rodriguez merged 7 commits intorsksmart:mainfrom
Ad-Capital:transaction-simulation
Jan 20, 2026
Merged

feat(cli): add simulate command for testing RBTC and ERC20 transfers …#288
ezequiel-rodriguez merged 7 commits intorsksmart:mainfrom
Ad-Capital:transaction-simulation

Conversation

@Ad-Capital
Copy link
Contributor

…with gas estimation, balance validation, cost analysis, and formatted result output

Copy link
Member

@scguaquetam scguaquetam left a comment

Choose a reason for hiding this comment

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

Please follow accordingly to current standard.

error?: string;
}

function logInfo(message: string) {
Copy link
Member

Choose a reason for hiding this comment

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

This is wrong implemented, in order for this feature of CLI to be used in the MCP Server, we need normalize the usage of console chalk and spinner, in a external/nonexternal basis, for that please refer to other existing commands and check how they use this, for example THIS where we use logMessage and startSpinner, succeedSpinner, etc.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the feedback. I've refactored simulate.ts to follow the standard pattern used in other commands

const balanceAfterTransfer = currentTokenBalance - transferAmount;
const networkName = isTestnet ? "Rootstock Testnet" : "Rootstock Mainnet";

console.log("\n" + chalk.cyan("📊 SIMULATION RESULTS") + "\n");
Copy link
Member

Choose a reason for hiding this comment

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

console.log management.

]
});
console.log(simulationTable);

Copy link
Member

Choose a reason for hiding this comment

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

these also

…rnal, switching to logMessage/logInfo/logError, simplifying function signatures, removing unused helpers, standardizing CLI vs external context handling, and cleaning up duplicate code
bin/index.ts Outdated
});

program.parse(process.argv);
program.parse(process.argv);
Copy link
Member

Choose a reason for hiding this comment

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

you have this repeated here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My bad

error?: string;
}

function logMessage(
Copy link
Member

Choose a reason for hiding this comment

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

In other PR you have implemented an improvement here with this management, is it okay if you do it here also?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Of course

@Ad-Capital
Copy link
Contributor Author

Ad-Capital commented Dec 14, 2025 via email

Copy link
Member

@scguaquetam scguaquetam left a comment

Choose a reason for hiding this comment

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

Please take in account these comments for better implementation.

const { balance: tokenBalance, decimals, name: tokenName, symbol: tokenSymbol } =
await getTokenInfo(publicClient, tokenAddress, walletAddress);

const currentTokenBalance = Number(tokenBalance) / (10 ** decimals);
Copy link
Member

Choose a reason for hiding this comment

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

Should we here use BigInt instead of Number? in case of managing too big values?
can we keep all in bigint?

async function simulateERC20Transfer(
params: TransactionSimulationOptions,
publicClient: any,
walletClient: any,
Copy link
Member

Choose a reason for hiding this comment

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

here you can use the PublicClient and WalletClient types from Viem instead of using any.

@@ -0,0 +1,162 @@
import { PublicClient, Account, Address, parseEther, formatEther } from "viem";
Copy link
Member

Choose a reason for hiding this comment

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

parseEther ist not used here

amount: bigint,
gasPrice?: bigint
): Promise<GasEstimationResult> {
let gasEstimate = BigInt(100000);
Copy link
Member

Choose a reason for hiding this comment

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

in this case, if the estimation fails, it is hardcoded, we should somehow , tell the user in this case this is a fallback or fixed value due to an error on estimation.

logMessage(isExternal, message, chalk.yellow);
}

export function capitalize(str: string): string {
Copy link
Member

Choose a reason for hiding this comment

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

I think this function is not being used on the project

Copy link
Contributor Author

Choose a reason for hiding this comment

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

had an idea for dynamic string formatting in the logger utilities and thought capitalize would be useful, but the idea didn't hold up after implementation, turns out hardcoded messages work better for this use case

logMessage(params.isExternal || false, summaryMessage, isSuccessful ? chalk.green : chalk.red);

return {
success: true,
Copy link
Member

Choose a reason for hiding this comment

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

here, even if the transaction simulation fails, you are returning a success true, and other fields.

- Replace any types with PublicClient/WalletClient from Viem
- Use BigInt for token balance calculations
- Add fallback gas warning for users
- Fix simulation result to return actual status instead of always true
- Remove unused capitalize function and parseEther import
- Add proper error logging and type guards
Add comprehensive documentation for the new simulate command that allows users to test RBTC and ERC20 token transfers before execution. Documentation includes usage examples, output samples, available options, and explanations of simulation results for both mainnet and testnet networks.
Copy link
Collaborator

@ezequiel-rodriguez ezequiel-rodriguez left a comment

Choose a reason for hiding this comment

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

LGTM

@ezequiel-rodriguez ezequiel-rodriguez merged commit 0c52dc9 into rsksmart:main Jan 20, 2026
3 checks passed
@ezequiel-rodriguez
Copy link
Collaborator

hey @Ad-Capital,
Thanks for your contribution! It looks really helpful 💪
Someone from the hacktivator program will contact you in order to get your reward 😄
Keep contributing 😉

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.

3 participants