go-spacemesh
go-spacemesh copied to clipboard
spacemeshos
vm: regular spawn is not supported
we don't support regular spawn in the current implementation.
as i understand there is big difference in how payload is encoded for spawn and self spawn:
- for self spawn: nonce is not included into payload. zeroes are returned by parse_payload
- for regular spawn: nonce needs to be included into payload, as it is a regular transaction from another principal
this difference is not reflected in the existing spec, so it might be that i don't understand something and instead skipped regular spawn altogether.
@dshulyak thanks for the heads up. I recently asked @noamnelke about this. I think it's best if he weighs in here.
This came up in https://github.com/spacemeshos/smapp/issues/888#issuecomment-1179341354
Isn't this asymmetry with nonce making it possible to spawn a lot of single-sig accounts using the same pubkey with different nonces, meanwhile with self-spawn it is possible to create only one single-sig account per pubkey? 🤔 Sounds weird to me. Did I miss something?
This sounds correct to me. The "spawn a lot of single-sig accounts using the same pubkey with different nonces" case simply corresponds to spawning many smart contracts from one principal account, which we definitely want to allow (just as one EOA can deploy any number of smart contracts in Ethereum). The "with self-spawn it is possible to create only one single-sig account per pubkey" case corresponds to the self-spawn special case where an account can receive funds before a wallet contract has been spawned there, something that can happen only once for a given keypair (just as there can only be one EOA in Ethereum per keypair).
since we are adding nonce to selfspawn, we can probably support this feature as well without significant effort? @lrettig do you want to include it?
As discussed on calls today, yes, we should support this at genesis if possible. Will leave it to you @dshulyak to figure out the details with @noamnelke. CC @selfdual-brain
Isn't this asymmetry with nonce making it possible to spawn a lot of single-sig accounts using the same pubkey with different nonces, meanwhile with self-spawn it is possible to create only one single-sig account per pubkey? 🤔 Sounds weird to me. Did I miss something?
@brusherru yes, you're missing something ;)
The account address is calculated by hashing the template address and the immutable state, whether or not the transaction that performs the spawn has a nonce is irrelevant (we're NOT hashing the entire transaction to determine the resulting account's address).
It is possible for a template developer to support spawning more than one account using the same immutable state, if that's desired, by allowing an additional nonce (or salt) inside the immutable state. There's no reason the platform should restrict one account instance per set of instantiation arguments.
@dshulyak when you have time to work on this again you should ping me and we'll discuss. I think that supporting both self-spawn and 3rd party spawn, while not using nonces in self-spawn is possible. We can work out any implementation details to get this to work.
@selfdual-brain thinks that redesigning the entire transaction spec is a better idea, but I disagree. I think that the design is good as-is. Regardless of a whether or not a better design can be created - this close to feature freeze, making a change like this feels risky and misguided to me.
I'm continuing to operate as if the existing TX design is what we're going to genesis with. Trying to find local solutions to any issues that come up.
i think parse_payload interface is not compatible with spawn (and remote execution in general). by design it parses two independent objects: header (max gas, gas price, nonce) and method call arguments. header (with one exception) is a property of the account that pays for gas. method arguments is a property of the account where method is executed.
to be more specific: header can be decoded by executing vesting wallet template, arguments are decoded by executing vault template.
it means that parse_payload needs to be split at least into parse_header and parse_args. to spawn a vault the sequence of parsing calls would be:
- vesting.parse_header
- vault.parse_args
however this is still not enough, since max_gas can't be computed by either of those methods. vesting knows the cost of the verification, but not the cost of execution and vice versa. so the max_gas must be decoded by two templates.
@selfdual-brain @noamnelke @lrettig
to be more specific: header can be decoded by executing vesting wallet template, arguments are decoded by executing vault template.
Yes, and this is actually our current design:
-
parsePayload()is supposed to decode the transaction into pieces -
methodArgumentsis one such piece - at this level of abstraction,
methodArgumentsis just an array of bytes.
Decoding methodArguments byta array into something more meaningful is the responsibility of the called method (regardless of this method being on local account or foreign account).
however this is still not enough, since max_gas can't be computed by either of those methods. vesting knows the cost of the verification, but not the cost of execution and vice versa. so the max_gas must be decoded by two templates.
Not quite. In Genesis we agreed on a fixed collection of templates and our fixed gas calculation is based on a hardcoded table. The knowledge on gas cost of the only "cross-contract method call" we have now - namely when VestingWallet drains a Vault - is also hardcoded in this table.
Your case is perfectly covered by the agreed gas computation:
val fixedGas = HARDCODED_TABLE(tx.template, tx.method)
val maxGas = fixedGas + tx.binarySize * STORAGE_COST_FACTOR
In the future, where a full SVM is in place, with gas metering working, this will of course work differently, but again - in the case of cross-contract calls, max-gas will be computed only by principal's template. The idea of max_gas is not to estimate the actual gas cost of to-be-performed execution, but to declare the limit of expense that the creator of transaction is willing to accept in the worst case. I guess that in 99% of cases this limit will be just statically declared in the transaction (like it currently happens in Ethereum).
Yes, we allow the template to compute this declaration dynamically - but this is just because our account abstraction was conceived to give all dimensions of freedom to the developer.
at this level of abstraction, methodArguments is just an array of bytes.
methodArguments can't be represented as array of bytes. even in your document there is no length prefix for method arguments. so it has to be decoded into concrete type.
Your case is perfectly covered by the agreed gas computation:
specifically spawn is covered only because it pays for:
- verify (fixed_gas)
- storage (tx.binarySize * STORAGE_COST_FACTOR)
drain vault is indeed covered because it is special transaction. but generic remote calls won't be covered. perhaps this is another reason not to try to implement them for genesis.
I agree with what @selfdual-brain said.
methodArguments can't be represented as array of bytes. even in your document there is no length prefix for method arguments. so it has to be decoded into concrete type.
It can be a fixed size byte array, since we can its length based on the entire transaction length minus known length fields. Does this solve the problem?
but generic remote calls won't be covered. perhaps this is another reason not to try to implement them for genesis.
I agree with you here, @dshulyak. There's also very little justification at this point, imo.
It can be a fixed size byte array, since we can its length based on the entire transaction length minus known length fields. Does this solve the problem?
i don't think that this proposal works. you won't know how to decode arguments if there is a variable length field after them (e.g in signature).
what works for me is to decode arguments either before parse_payload or after. @selfdual-brain suggested to length prefix them and decode in parse_payload, thats another option.. and there are not so many downsides beside it being ugly.
i need to finalize change to parse_payload (if any), there are three options:
- split parse_payload into parse_envelope and parse_args. parse_envelope is called on principal account template, parse_args is called on the account that is spawned. the difference here is that arguments are not deferred anymore and decoded in one sequence.
- length prefix arguments, so that they can be decoded into concrete struct in a deferred mode. parse_payload will read length, and decode arguments into byte slice.
- assume that arguments can be decoded based on signature length. if signature is constant size parse_payload can decode everything before it as arguments. for default templates we don't need to worry about variable sized signatures.
from implementation pov everything works. if we can agree that wasm will use 3rd option, then i don't need to change anything and can proceed with merging spawn and vesting
The preferred way to represent method call arguments and spawn arguments (most coherent with the overall architecture) is to keep them as Seq[Byte] structure at the tx envelope layer.
The decoding of what is inside this Seq[Byte] just belongs to a different layer.
This interpretation does not overlap with any of 3 variants you proposed above. Technically it is most similar to variant (2), because the representation of Seq[Byte] at Scala Codec level is that the length of the sequence is encoded.
From the perspective of the new spec I am now preparing
this stmt scares me. new spec? 3 months before planned genesis? why?
My original plan was to do (3), this has the advantage of making transactions smaller (no need to encode the arguments length), but the downside is that you must know the length of the validation data when parsing the payload. While this is not impossible, it adds some complexity, especially when considering 3rd party templates (template devs would need to provide the validation data length which is non-obvious and might be limiting some use-cases).
I'm increasingly of the opinion that this is one place where saving a few bytes is not worth the added complexity.
I don't have a strong preference between (1) and (2) so if @selfdual-brain thinks that (2) is better and @dshulyak doesn't think it's harder to implement or maintain than (1), then let's do that.