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

Updated gas #55

Merged
merged 1 commit into from
Feb 16, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
23 changes: 15 additions & 8 deletions apps/astraplusplus/widget/DAO/Proposal/Create/FunctionCall.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ State.init({
method_name: state.method_name,
args: state.args || "{}",
deposit: state.deposit || "0",
gas: "50000000000000",
gas: "270",
error: undefined,
receiver_id: null,
description: null,
Expand Down Expand Up @@ -107,9 +107,16 @@ const handleFunctionCall = () => {
});
return;
}
if (state.gas > 300) {

Choose a reason for hiding this comment

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

as mentioned 300 TGas will always fail. It's difficult to give an exact estimate, but this is the gas that gets attached to the cross contract call, so it's always less than 300 TGas. I would reduce the max allowed Gas here to the default 270

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Quick question
Is it necessary it will always be a cross contract call.?

Choose a reason for hiding this comment

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

Yes that's by design of Sputnik DAO contract

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I will update it, thanks for the information

State.update({
error: "Maximum gas allowed is 300Tgas"
});
return;
}
}

const deposit = Big(state.deposit).mul(Big(10).pow(24)).toFixed();
const gas = Big(state.gas).mul(Big(10).pow(12)).toFixed();
if (isVotingBodyDao) {
if (isEmpty(state.description)) {
State.update({
Expand All @@ -133,7 +140,7 @@ const handleFunctionCall = () => {
method_name: state.method_name,
args: fc_args,
deposit: deposit,
gas: state.gas ?? "50000000000000"
gas: gas
}
]
}
Expand Down Expand Up @@ -193,7 +200,7 @@ const handleFunctionCall = () => {
"utf-8"
).toString("base64"),
deposit: deposit,
gas: state.gas ?? "50000000000000"
gas: gas
}
]
}
Expand Down Expand Up @@ -240,7 +247,7 @@ const handleFunctionCall = () => {
method_name: state.method_name,
args: fc_args,
deposit: deposit,
gas: state.gas ?? "50000000000000"
gas: gas
}
]
}
Expand Down Expand Up @@ -271,7 +278,7 @@ const handleFunctionCall = () => {
method_name: state.method_name,
args: fc_args,
deposit: deposit,
gas: state.gas ?? "50000000000000"
gas: gas
}
]
}
Expand Down Expand Up @@ -504,19 +511,19 @@ return (
)}
{!isCongressDaoID && !isVotingBodyDao && (
<div className="mb-3">
<h5>Gas</h5>
<h5>Gas (Tgas)</h5>
<input
type="number"
value={state.gas}
onChange={(e) => onChangeGas(e.target.value)}
defaultValue="300000000000000"
defaultValue="270"
/>
</div>
)}
</>
)}
<div className="mb-3">
<h5>Deposit {(isCongressDaoID || isVotingBodyDao) && "(NEAR)"}</h5>
<h5>Deposit (NEAR)</h5>
<input
type="number"
value={state.deposit}
Expand Down
2 changes: 1 addition & 1 deletion apps/astraplusplus/widget/DAO/Proposals/Card/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -493,7 +493,7 @@ const handleVote = ({
contractName: daoId,
methodName: isCongressDaoID ? "vote" : "act_proposal",
args: args,
gas: 200000000000000
gas: 270000000000000

Choose a reason for hiding this comment

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

the act_proposal can have max gas attached

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Are you sure.? I m not much aware about the contract side complications with different proposal types, but I would rather have it set a bit less, if it allows almost all of the use cases.

Choose a reason for hiding this comment

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

Most proposals need way less gas. There's no downside in attaching max because you get refund anyway.

That's why it would be best to have an input field for attached gas or maybe also linked to a slider.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Okay I will increase it to 300Tgas. The reason I don’t want to display an input or slider in UI is most people who aren’t aware will get confused, and may be assign too little gas (in hope of saving some), and the txn might fail because of that. We can revisit it later.

}
];
if (showNotification) {
Expand Down
Loading