-
Notifications
You must be signed in to change notification settings - Fork 116
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
add simulateTransaction endpoint #1610
Changes from 4 commits
45749b1
23c8b1d
65df47b
6fc64c9
693b7c0
0da1d84
1a2e340
8253c81
7d26831
75c07e6
a306b46
5379398
ff86ecf
f42b284
c08b8cf
793a6b0
7f3868e
5399ff3
c172c59
89a134d
f5d4dd8
e3855fe
9c74334
267e13b
b6bd4f2
e51d8fd
689d0ec
8f020d8
65c9c42
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,6 +18,7 @@ import ( | |
"github.com/ava-labs/hypersdk/codec" | ||
"github.com/ava-labs/hypersdk/consts" | ||
"github.com/ava-labs/hypersdk/fees" | ||
"github.com/ava-labs/hypersdk/state" | ||
"github.com/ava-labs/hypersdk/state/tstate" | ||
) | ||
|
||
|
@@ -235,3 +236,96 @@ func (j *JSONRPCServer) Execute( | |
|
||
return nil | ||
} | ||
|
||
type SimulateTransactionArgs struct { | ||
Tx []byte `json:"tx"` | ||
} | ||
|
||
type SimulatedAction struct { | ||
Output []byte `json:"output_b64"` | ||
Error *string `json:"error"` | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could we use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done |
||
|
||
type SimulateTransactionReply struct { | ||
SimulatedActions []SimulatedAction `json:"simulatedActions"` | ||
ReadKeys []string `json:"readKeys"` | ||
WriteKeys []string `json:"writeKeys"` | ||
AllocateKeys []string `json:"allocKeys"` | ||
BlockHeight uint64 `json:"blockheight"` | ||
Error string `json:"error"` | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could we reduce the state key fields here down to just There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd prefer to remove There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done |
||
|
||
type SimulationEnabledRules struct { | ||
chain.Rules | ||
} | ||
|
||
func (SimulationEnabledRules) GetTransactionExecutionMode() chain.TransactionExecutionMode { | ||
return chain.SimulatedTransactionExecution | ||
} | ||
|
||
func (j *JSONRPCServer) SimulateTransaction( | ||
req *http.Request, | ||
args *SimulateTransactionArgs, | ||
reply *SimulateTransactionReply, | ||
) error { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could we start with simulating a single action and providing the additional arguments as needed (actor) ? In our case, transactions include additional data that we don't need to require from the user. For example, with this API you can only simulate an action from a given address if you can generate a signature from that address. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done |
||
ctx, span := j.vm.Tracer().Start(req.Context(), "JSONRPCServer.SimulateTransaction") | ||
defer span.End() | ||
|
||
if reply == nil { | ||
return errors.New("SimulateTransaction was called with a nil reply object") | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should be provided by the server, so we can remove this check and follow the style in the rest of the APIs of not checking if the reply value is nil. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done |
||
|
||
actionRegistry, authRegistry := j.vm.ActionRegistry(), j.vm.AuthRegistry() | ||
rtx := codec.NewReader(args.Tx, consts.NetworkSizeLimit) // will likely be much smaller than this | ||
tx, err := chain.UnmarshalTx(rtx, actionRegistry, authRegistry) | ||
if err != nil { | ||
return fmt.Errorf("%w: unable to unmarshal on public service", err) | ||
} | ||
if !rtx.Empty() { | ||
return errors.New("tx has extra bytes") | ||
} | ||
actor := tx.Auth.Actor() | ||
|
||
currentState, err := j.vm.ImmutableState(ctx) | ||
if err != nil { | ||
return err | ||
} | ||
reply.BlockHeight = j.vm.LastAcceptedBlock().Hght | ||
recorder := state.NewRecorder(currentState) | ||
simulationEnabledRules := SimulationEnabledRules{j.vm.Rules(tx.Base.Timestamp)} | ||
for actionIdx, action := range tx.Actions { | ||
actionOutput, err := action.Execute(ctx, simulationEnabledRules, recorder, tx.Base.Timestamp, actor, chain.CreateActionID(tx.ID(), uint8(actionIdx))) | ||
|
||
var simAction SimulatedAction | ||
if actionOutput == nil { | ||
simAction.Output = []byte{} | ||
} else { | ||
simAction.Output, err = chain.MarshalTyped(actionOutput) | ||
if err != nil { | ||
return fmt.Errorf("failed to marshal output: %w", err) | ||
} | ||
} | ||
if err != nil { | ||
errString := err.Error() | ||
simAction.Error = &errString | ||
} | ||
|
||
reply.SimulatedActions = append(reply.SimulatedActions, simAction) | ||
|
||
if err != nil { | ||
break | ||
} | ||
} | ||
for key, perm := range recorder.GetStateKeys() { | ||
if perm.Has(state.Read) { | ||
reply.ReadKeys = append(reply.ReadKeys, key) | ||
} | ||
if perm.Has(state.Write) { | ||
reply.WriteKeys = append(reply.WriteKeys, key) | ||
} | ||
if perm.Has(state.Allocate) { | ||
reply.AllocateKeys = append(reply.AllocateKeys, key) | ||
} | ||
} | ||
return nil | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -25,6 +25,13 @@ type ( | |
ActionRegistry *codec.TypeParser[Action] | ||
OutputRegistry *codec.TypeParser[codec.Typed] | ||
AuthRegistry *codec.TypeParser[Auth] | ||
|
||
TransactionExecutionMode int | ||
) | ||
|
||
const ( | ||
OnchainTransactionExecution TransactionExecutionMode = iota | ||
SimulatedTransactionExecution | ||
) | ||
|
||
type Parser interface { | ||
|
@@ -152,6 +159,8 @@ type Rules interface { | |
GetStorageValueWriteUnits() uint64 // per chunk | ||
|
||
FetchCustom(string) (any, bool) | ||
|
||
GetTransactionExecutionMode() TransactionExecutionMode // either OnchainTransactionExecution or SimulatedTransactionExecution | ||
} | ||
|
||
type MetadataManager interface { | ||
|
@@ -229,8 +238,8 @@ type Action interface { | |
// Execute actually runs the [Action]. Any state changes that the [Action] performs should | ||
// be done here. | ||
// | ||
// If any keys are touched during [Execute] that are not specified in [StateKeys], the transaction | ||
// will revert and the max fee will be charged. | ||
// If any keys are touched during [Execute] while running in [OnchainTransactionExecution] mode that | ||
// are not specified in [StateKeys], the transaction will revert and the max fee will be charged. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm this comment was actually not true before. If a state key is read that was not specified, it's up to the action to decide whether to propagate it (usually should). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could we revert this if we're able to remove There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done. |
||
// | ||
// If [Execute] returns an error, execution will halt and any state changes will revert. | ||
Execute( | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -101,7 +101,7 @@ func (j *JSONRPCServer) simulate(ctx context.Context, t actions.Call, actor code | |
if err != nil { | ||
return nil, 0, err | ||
} | ||
recorder := storage.NewRecorder(currentState) | ||
recorder := state.NewRecorder(currentState) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we remove this from There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done |
||
startFuel := uint64(1000000000) | ||
callInfo := &runtime.CallInfo{ | ||
Contract: t.ContractAddress, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -152,6 +152,10 @@ func (*Rules) FetchCustom(string) (any, bool) { | |
return nil, false | ||
} | ||
|
||
func (*Rules) GetTransactionExecutionMode() chain.TransactionExecutionMode { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there a reason that we need to add this mode to If we just wrap the current view produced by the VM and run There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done. |
||
return chain.OnchainTransactionExecution | ||
} | ||
|
||
type ImmutableRuleFactory struct { | ||
Rules chain.Rules | ||
} | ||
|
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.
Can we re-use the types specified in
server.go
as we do for the existing server/client implementations?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.
done