-
Notifications
You must be signed in to change notification settings - Fork 3
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
Updated gas #55
Conversation
@@ -493,7 +493,7 @@ const handleVote = ({ | |||
contractName: daoId, | |||
methodName: isCongressDaoID ? "vote" : "act_proposal", | |||
args: args, | |||
gas: 200000000000000 | |||
gas: 270000000000000 |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
@@ -107,9 +107,16 @@ const handleFunctionCall = () => { | |||
}); | |||
return; | |||
} | |||
if (state.gas > 300) { |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.?
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
Resolves #33
Resolves #34