ARC56: Extended App Description #258

joe-p · 2023-11-15T00:28:14Z

Basically takes most of the ideas in ARC32 and reorgs it in a way that is backwards compatible with ARC4 clients. Main benefit of this is HLLs can simply generate one JSON artifact.

ARCs/arc-draft_extended_desc.md

robdmoore · 2023-11-15T07:19:17Z

ARCs/arc-draft_extended_desc.md

+     * information about the deployed contract in the network indicated by the
+     * key
+     */
+    [network: string]: {


Suggested change

[network: string]: {

[network: 'mainnet' | 'testnet' | 'betanet' | string]: {

Current one is base64 genesis hash, but I feel like we should support the more understandable short-hand?

Side-note: just realised this is part of ARC-0004. I've never seen anyone use this, it's a bit tricky if you are auto-generating the json file to include stateful data like this. We also don't currently support it in algokit utils, although if we want to support it we should add that! This would be handy when third parties like oracles are distributing a file.

Part of the rationale for the base64 is that is allows one to accurately describe a private network. We could have a separate entry for public networks though, but because this is so seldomly used I'm not sure if it'd really be worth it.

My suggestion above still has string so the implementation would be it's either base64 or one of the special known network names. It would make it a lot easier to use this feature.

This is minor though happy if it's left out.

Yeah that works with me. I'll just specify in the ARC that the human names are NOT backwards compatible and MUST be accompanied by the genesis hash as well in the object with the same app IDs.

robdmoore · 2023-11-15T07:20:13Z

ARCs/arc-draft_extended_desc.md

+     */
+    [network: string]: {
+      /** The app ID of the deployed contract in this network */
+      appID: number;


Worth adding in the AlgoKit format as well as another option? i.e. creator address + name

I'm trying to think how this could be done in a way that is ARC4 backwards compatible. Don't want to omit appID because that could break an ARC4 client if it tries to use that field (no clients in existence use it that I know of, but just want to be safe)

If nothing actually uses this right now then I think it's OK to mess with it and note is as a known deviation from ARC4?

Again, pretty minor feature though.

If possible, since we tried to keep ARC4 backward compatibility, I would not pursue this minor deviation. I think declaring the association between appID and the Network ID on which the App is deployed has some value.

robdmoore · 2023-11-15T07:21:00Z

ARCs/arc-draft_extended_desc.md

+  readonly: boolean;
+}
+
+interface Contract {


Given this is the driving interface suggest this is added at the top of the code snippet, possibly also worth having a comment to indicate this is an extension of ARC4?

Added comments in a18a968. I used this ordering because it ensures that any interface is described before it's actually used, which makes more sense to me but happy to change it if others feel otherwise.

I think it's a bit hard to comprehend right now. I'd advocate for splitting it out into multiple definitions and indicate with narrative that the definitions are further down the page.

Let me know what you think of the recent update

ARCs/arc-draft_extended_desc.md

robdmoore · 2023-11-15T07:45:25Z

ARCs/arc-draft_extended_desc.md

+  /** The type of the key. Can be ABI type or named struct */
+  keyType: string;
+  /** The type of the value. Can be ABI type or named struct */
+  valueType: string;


Wonder if this should be StructName | ABIType or similar?

Also, previously we had avm type, do we need to support that too / instead? Having the proper type would be awesome because it's an annoying limitation of the current stuff.

Wonder if this should be StructName | ABIType or similar?

Yeah I like that. Will make the change.

Also, previously we had avm type, do we need to support that too / instead? Having the proper type would be awesome because it's an annoying limitation of the current stuff.

Initially I was thinking it wasn't necessary because it's obvious when one is using uint64 vs ABI uint64, but we do need a way to specifcy AVM bytes (no prefix) vs ABI bytes. i'll add AVMType as well.

I added AVMBytes type in a18a968. I don't think uint64 is needed because it's always going to be obvious when AVM uint64 is used (only top-level values in global/local)

ARCs/arc-draft_extended_desc.md

robdmoore · 2023-11-15T07:47:37Z

ARCs/arc-draft_extended_desc.md

+      };
+    };
+    /** Describes single key-value pairs in the application's state */
+    keys: {


Previous one had reserved and declared values, I've honestly never used reserved, but I guess it makes sense where you want to have an upgradeable contract that preallocates more storage than it currently needs?

Same re: static that was there previously, do we need it?

I didn't make a differentiation between reserved and declared because schema is immutable, thus it would make sense to always just use the max amount of state when creating the app (declared + reserved).

Same re: static that was there previously, do we need it?

I don't see static in ARC32 itself. How is it used?

Yeah I don't think static is used so I'm happy to omit

I wonder if a better way then is to allow for you to override the number of ints/bytes rather than having reserved (which I always thought was weird).

ARCs/arc-draft_extended_desc.md

robdmoore · 2023-11-15T07:55:00Z

ARCs/arc-draft_extended_desc.md

+## Specification
+```ts
+/** Mapping of named structs to the ABI type of their fields */
+interface StructFields {


This one is an associative array, but all of the other things seem to be arrays with objects that have a name property. Should we be consistent?

I'm not sure I follow, what do you mean?

i.e. this one is:

{ fieldName: ... }

rather than the others parts of the spec that are:

[{name: "fieldName", ...}]

ARCs/arc-draft_extended_desc.md

robdmoore · 2023-11-15T08:10:02Z

I know the eventual goal is to use simulate, but I wonder if we add in optional per-method hints for things like:

Fees
Boxes
Foreign arrays (? where you don't want that in the method selector)

And also:

Template variables?

Are there other things that have come up in the past worth considering while we are looking at this?

joe-p · 2023-11-15T10:54:06Z

I know the eventual goal is to use simulate, but I wonder if we add in optional per-method hints for things like:

Fees
Boxes
Foreign arrays (? where you don't want that in the method selector)

My rationale for not including a field for these sort of things is that they can be dynamic which can be hard to describe in JSON and they could be network dependent. It also is not easy for a HLL to autogenerate these sorts of things and if the dev is required to manually write them out might as well just have them write a standardized readonly method that returns this information so they have the full power of the language and contract state/methods at their disposal (see ARC51).

Template variables?

Yes good call.

Co-authored-by: Rob Moore (MakerX) <robertmooreweb@gmail.com>

joe-p · 2024-04-30T11:09:33Z

@SudoWeezy I think we are ready to merge and move to last call

achidlow · 2024-05-09T06:48:35Z

ARCs/arc-0056.md

+  /** Description of what this storage key holds */
+  desc?: string;
+  /** The type of the key */
+  keyType: ABIType | AVMBytes | StructName;


Why does StorageKey have keyType? Isn't this for singular, known values?

Yes but I figured it's still useful to know the type for things like explorers and frontends

So it's intended to indicate the encoding of key? Maybe keyEncoding would be clearer? I don't think it benefits from consistency of naming with StorageMap.keyType since that seems of a different purpose - to indicate what sort of value to encode to combine with StorageMap.key in order to retrieve a value. Whereas StorageKey.key is already encoded and you may want to know how to interpret that value. So they're kind of opposites?

emg110 · 2024-05-09T16:34:53Z

This is a much-needed ARC and everything looks great. When merge?

joe-p · 2024-05-10T00:37:08Z

This is a much-needed ARC and everything looks great. When merge?

Very close. Initial generation and consumption of ARC56 has been spiked out and things look good so far. Unless something is caught during Puya integration I think this is ready to be finalized

achidlow · 2024-05-10T01:32:43Z

ARCs/arc-0056.md

+  /** The type of the values in the map */
+  valueType: ABIType | AVMBytes | StructName;
+  /** The prefix of the map, encoded as a utf-8 string */
+  prefix?: string;


Why utf-8 here and not base64 like StorageKey.key? UTF-8 is more readable, but does that not preclude raw byte prefixes?

Good point. I can change to base64

ARCs/arc-0056.md

achidlow · 2024-05-10T01:43:39Z

ARCs/arc-0056.md

+  /** The pre-compiled TEAL that may contain template variables. MUST be omitted if included as part of ARC23, but otherwise MUST be defined. */
+  source?: {
+    /** The approval program */
+    approval: string;


can we make this be the TEAL or the bytecode?

Are you saying have two properties? One for pre-compiled TEAL and one for compiled bytecode?

achidlow · 2024-05-10T01:48:20Z

ARCs/arc-0056.md

+  };
+  /** ARC-28 events that MAY be emitted by this contract */
+  events?: Array<Event>;
+  /** A mapping of template variable names as they appear in the teal (not including TMPL_ prefix) and their respecive types */


There doesn't seem to be any harm if we include the TMPL_ prefix? The less assumptions (within the bounds of practicality) the better, no?

But if we use it for code generation then you’d have to strip the TMPL so we have to cater for it somewhere?

If certain tooling expects that as a prefix, that seems like the place to check for and strip that prefix, yeah?

shrug could go either way, I guess it's fair that this could be more general purpose and then the generators are clever enough to detect and strip TMPL_ when emitting code...

My rationale omitting the TMPL_ here and forcing tooling to use it is to make it easier to implement safely. For example, if ARC56 allowed anything as the variable name and the tooling didn't have to add TMPL_, it would need to check all the variable names against all opcodes. Otherwise an ARC56 file could have a template variable named byte which subsequently overwrites all byte opcodes which is not the intended effect of template variables.

ARCs/arc-0056.md

achidlow · 2024-05-10T02:03:58Z

ARCs/arc-0056.md

+  /** The arguments of the event, in order */
+  args: Array<{
+    /** The type of the argument */
+    type: ABIType;


Since this isn't part of ARC-4, maybe in the case of a struct, we can avoid re-specifying it as a tuple here?

It is not a part of ARC4 but it is a part of ARC28. You could make the argument that ARC28 doesn't have much adoption yet, but I think it's better to keep backwards compatibility with existing ARCs

achidlow · 2024-05-10T06:12:02Z

So there's potentially three different kinds of stages that we can think about here:

A contract with deploy-time template variables
A contract that has been compiled to byte code and is ready to be deployed.
A contract that is deployed

In scenario one, clearly the TEAL is needed, as well as the template variables available. It's not possible to have sourceInfo populated at this point, since PC values might change if there are byte string template variables.

In scenario two, there is sourceInfo available, and the corresponding byte code should be embedded, to guarantee correct PC mappings. Also, pc should be non-empty.

In scenario three, the network -> appID mapping can be populated. Bytecode is perhaps optional, but sourceInfo should be retained (but optional).

Some other things worth mentioning:
Puya has support for compiling directly to byte code (ie offline compilation) which should get merged shortly, and in that case, there isn't necessarily any TEAL source. Maybe including higher level language source in the source info is an option instead?
Naturally, it can still output the TEAL in any case - and if there are template variables that aren't specified at compile time, byte code can't be produced.
There's some additional size optimisations we've included, so the byte code might not match exactly what algod produces (although you can disable these optimisations to get it to match).

joe-p · 2024-05-10T11:43:32Z

A contract that is deployed

I've also realized in this scenario we should include the template variables used as well so the source map can be reconstructed.

robdmoore · 2024-05-10T11:58:25Z

Makes sense (as an optional thing). It may be that you want to distribute an app spec for consumers without source code so general public can consume it so all of the source stuff needs to be optional.

joe-p · 2024-05-10T12:03:16Z

Makes sense (as an optional thing). It may be that you want to distribute an app spec for consumers without source code so general public can consume it so all of the source stuff needs to be optional.

I don't feel strongly but one reason to enforce it to exist is to preserve error messages

robdmoore · 2024-05-10T12:25:01Z

You could include PC => error message only though in that case?

joe-p · 2024-05-11T12:22:24Z

One thing that I also want to add that we've missed: scratch slots.

joe-p · 2024-05-20T23:28:10Z

Just did the following

Add scratch variables
Add a optional value key to template variables to see what template variables were used for a deployment
Added an optional byteCode field and made teal optional in SourceInfo

extended app description draft

ef5f338

github-actions bot added c-new s-draft t-arc labels Nov 15, 2023