ARC56: Extended App Description

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.

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?

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.

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

Three points on my side:

Maybe suggested default values for optional arguments? Examples: default value for argument foo of Bar method is equal to the App  Global Var spam (I think default values is something that Beaker tried to support already);
There were past discussions about moving reference types from ARC-4 to an upper level (e.g. ARC-32). Is this still relevant?
There were also past discussions about an incompatibility of ARC-4 with AVM 9 pooled resources. Should this spec address this?

Maybe suggested default values for "optional" arguments? Examples: default value for argument foo of method Bar is equal to the App Global Var spam (I think default values is something that Beaker tried to support already);

Yeah ARC32 has this but I've never seen it really used. I'm fine with adding static default arguments but I think it can get a bit complicated when we try to use things like state, methods, etc. Perhaps arguments could be described in ARC51 events rather than in JSON to enable fully dynamic argument requirements.

There were past discussions about moving reference types from ARC-4 to an upper level (e.g. ARC-32). Is this still relevant?
There were also past discussions about an incompatibility of ARC-4 with AVM 9 pooled resources. Should this spec address this?

It was eventually decided that it is not worth removing the reference types from the ABI for the sake of backwards compatibility. I think in the near future simulate can be used to solve most, if not all resource availability problems.

re: fees, boxes, foreign arrays
Agreed they can be dynamic, but sometimes they aren't and it is really nice to be able to provide the hint so it can handled automatically by a generated client.
The voting contract is such a good idea, there are very clear box and fee semantics. Noting that one of the boxes is dynamic so in that case I'd just want to specify you have to pass a box value in with a name of voter_address and then the generated client can prompt for that in the method call.

Agreed they can be dynamic, but sometimes they aren't and it is really nice to be able to provide the hint so it can handled automatically by a generated client.

My thinking is that how things are referenced might change in a future AVM version (much like the changes in AVM9), thus we'd need to make changes to how they are defined in the JSON which adds more complexity to all the clients.
We have unnamed resources support in simulate right now, it's just a matter of adding a function to the SDKs to auto-populate the arrays and then devs never have to worry about it. My plan is to work on said function starting this week.

It was eventually decided that it is not worth removing the reference types from the ABI for the sake of backwards compatibility. I think in the near future simulate can be used to solve most, if not all resource availability problems.

A bit off-topic: @joe-p does this means that an eventual "state-of-art" guideline should discourage the usage of reference types (i.e. considering them deprecated while still supported for backward compatibility)?

A bit off-topic: @joe-p does this means that an eventual "state-of-art" guideline should discourage the usage of reference types (i.e. considering them deprecated while still supported for backward compatibility) and rely on simulate "discovery/autocomplete" at AppCall time?

Yeah exactly, which is why I don't think it's important to include them in this spec. The idea is that this to-be-created function will be much more space efficient when packing foreign arrays compared to manually setting them (through ABI reference types or otherwise)

I was having a further think about this today.
Sometimes it's possible to statically determine things like foreign references, box refs, fees, etc. In other cases it's more obscured and dynamic.
I think that for the statically determined cases it's worth allowing this spec to specify them and indicate they have been determined and then if they can't be statically determined they are left out and instead the client looks up simulate first. In this way we aren't creating a situation where for every single smart contract call you have to issue a second HTTP call (which is very inefficient and slow).
The work we are doing with the Wyvern compiler will be able to determine static usage and so this will be able to be emitted in an ARC-32 like file.

@joe-p @robdmoore I was thinking about the current state of Smart Contract errors and failure paths' UX.
I think that a comprehensive AppSpec, which describes how to "interact" with the Smart Contract, should also describe the possible failures and error messages. This would enable a "complete" App client, allowing recovery actions and improving the overall App UX in case of failures.
Releasing App's SourceMap along with the AppSpec described by this ARC would be possible to include in the AppSpec described in this ARC a standardised way to manage App's errors (i.e. error code and description associated to a given Program Counter).
WDYT?

Something else to consider, it would be nice to have an extensibility point around different ways of encoding data. Currently this spec is using an ABI encoding as specified in ARC4. However it might be more flexible to allow describing types independently from their encoding, and then also have an encoding field which can be used to reference an encoding spec, initially this might just be ARC4 ABI, but could allow other specifications down the track

I think that for the statically determined cases it's worth allowing this spec to specify them and indicate they have been determined and then if they can't be statically determined they are left out and instead the client looks up simulate first.

If  how references, fees, or MBR work changes, then existing specs would break and the ARC itself might need to be updated. For example, if a contract requires 0.1A for an ASA opt-in and then the ASA opt in cost is reduced, the spec will now overcharge users. Even worse, if the fees or MBR amounts were to ever increase specs might be completely broken.
Also, hardcoding references prevents them from being efficiently packed. For example, if an app require's Alice's account but Alice is receiving an unrelated payment in the same transaction group, having the foreign reference for Alice's account is redundant and that space could potentially be used by another unrelated app call.

In this way we aren't creating a situation where for every single smart contract call you have to issue a second HTTP call (which is very inefficient and slow).

I think if someone is that concered about the latency of the extra HTTP calls they are better off not using an app client anyways and hardcoding the most opimized transactions they can (ie. caching suggested params, manually filling references, etc.).
If we really want to reduce HTTP calls when using the app spec, then I think we need to think about changes upstream in go-algorand, like removing references from the signature so algod can fill in the references on a signed transaciton.

I think that a comprehensive AppSpec, which describes how to "interact" with the Smart Contract, should also describe the possible failures and error messages. This would enable a "complete" App client, allowing recovery actions and improving the overall App UX in case of failures.

Good thinking. This is something I've always thought about and makes a ton of sense to be included here. A basic mapping of PC to string will go a long way in debugging/error handling. I will update the ARC tomorrow to include this.
This also makes me think we should include source mapping. The main two options are VLQ encoding and a json schema. VLQ being more space efficient and JSON being human readable. Personally I switched TEALScript from emitted VLQ to JSON because of the human readability, but in a future with debuggers and better srcmap tooling that might be a moot point.

Something else to consider, it would be nice to have an extensibility point around different ways of encoding data. Currently this spec is using an ABI encoding as specified in ARC4. However it might be more flexible to allow describing types independently from their encoding, and then also have an encoding field which can be used to reference an encoding spec, initially this might just be ARC4 ABI, but could allow other specifications down the track

To make sure I am understanding you correctly, you are basically advocating for type aliases. Like I can have
{ "timestamp": "uint64" }

Or something else?

If how references, fees, or MBR work changes, then existing specs would break and the ARC itself might need to be updated. For example, if a contract requires 0.1A for an ASA opt-in and then the ASA opt in cost is reduced, the spec will now overcharge users. Even worse, if the fees or MBR amounts were to ever increase specs might be completely broken.

I think these are straw~man~person arguments - if MBR or fee costs are changed a lot of things would break all over the ecosystem, those numbers are hard-coded everywhere. That's not a great thing obviously, and maybe there is still we can do with this spec to offset that risk, but I don't think that's the reason not to do it.

Also, hardcoding references prevents them from being efficiently packed. For example, if an app require's Alice's account but Alice is receiving an unrelated payment in the same transaction group, having the foreign reference for Alice's account is redundant and that space could potentially be used by another unrelated app call.

That's fair, but how often is that an issue? Plus, in the rare case it is an issue presumably, we'd always include an escape hatch to override the static defaults.

I think if someone is that concerned about the latency of the extra HTTP calls they are better off not using an app client anyways

It's not so much about that, but general distributed systems theory - the more external calls there are the more the chance of things going wrong, slow/poor UX, etc. If we can determine with the smart contract compiler when there are dynamic references and a call is needed, but otherwise we don't then in my opinion that's only a good thing.

hardcoding the most opimized transactions they can (ie. caching suggested params, manually filling references, etc.).

That should always be an option, it's important to follow the AlgoKit modularity principle and provide escape hatches in my opinion, but I'm more concerned here about helping devs fall into the pit of success (all facets: correctness, speed, UX, etc.).

I think that a comprehensive AppSpec, which describes how to "interact" with the Smart Contract, should also describe the possible failures and error messages.

+1 definitely would be good to include that. On a similar vein, does it also make sense to note any events that may get emitted?

I think these are strawmanperson arguments - if MBR or fee costs are changed a lot of things would break all over the ecosystem, those numbers are hard-coded everywhere. That's not a great thing obviously, and maybe there is still we can do with this spec to offset that risk, but I don't think that's the reason not to do it.

Just because there is already code out there that will break if these things happen doesn't mean we should introduce a standard that encourages developers to deploy even more code that will break.

That's fair, but how often is that an issue? Plus, in the rare case it is an issue presumably, we'd always include an escape hatch to override the static defaults.

It's not often an issue because there are hardly any code that actually use group resource sharing and unamed resources because the tooling for it does not exist. This is our opportunity to write a standard and tooling that leverages these great features of the AVM/algod.

It's not so much about that, but general distributed systems theory - the more external calls there are the more the chance of things going wrong, slow/poor UX, etc.

I agree, but a standard that encourages bad practices is not the best way to go about solving the problem.

That should always be an option, it's important to follow the AlgoKit modularity principle and provide escape hatches in my opinion

The same could be said the other way around. The spec doesn't define hardcoded resources, but algokit could allow users to manually specify them. If a dev cares about reducing calls and optimizing their flow, then they'll be writing custom code anyways.

+1 definitely would be good to include that. On a similar vein, does it also make sense to note any events that may get emitted?

Ah yes, I'll need to add ARC28 to this interface

@robdmoore Just thought of a happy medium. Instead of the spec including the references, it just describes the resources. For example, if a app reads 10k from box "foo", the spec will contain something like "boxes": { "foo": {"read": 10_000}} (and has have fields for write, create, and expand). Similar instead of writing the fees required, it just describes how many inner transactions are sent. This approach might eventually result in unused data in the JSON, but never incorrect data.

Just thought of a happy medium. Instead of the spec including the references, it just describes the resources. For example, if a app reads 10k from box "foo", the spec will contain something like "boxes": { "foo": {"read": 10_000}} (and has have fields for write, create, and modify size). Similarly instead of writing the fees required, it just describes how many inner transactions are sent. And for MBR changes, it describes the actions (ie. box create, box delete, optin/out). This approach might eventually result in unused data in the JSON, but never incorrect data.

Yes! That's exactly what I was thinking :)

Noting that if the static values can't be calculated because for instance there are loops, then it should indicate there are dynamic resources and you would use simulate endpoint by default (but could also allow someone to specify the dynamic amount if they wanted to skip the extra http request.

Similarly instead of writing the fees required, it just describes how many inner transactions are sent.

@joe-p, @robdmoore I'm not sure I'd want to go down this path.
The number of inner transactions triggered / foreing resources touched by an AppCall can not be statically predicted, since they could depend on the execution path.
I think would be very painful for any tool to come up with a proper automated construction of the AppSpec in this case. I'm more inclined to keep this AppSpec (quite big already) as close as possible to a "statically completable" companion of the App.
I would rather not like to see in the AppSpec not-so-informative manually-added fileds that just say: "this couldn't be statically derived, please simulate it".
I rather much prefer to keep the AppSpec simple, restricting it only to all those useful information "statically" deducible from the App definition (i.e. from PyTeal/TealScript/etc., from TEAL source code and from SourceMap).

Just adding some TODO's:

[ ] Add source mapping to spec
[ ] Add ARC28 to spec
[ ] Change error mapping from PC to TEAL (so HLLs don't NEED to use algod to get pc mapping)

Something else to consider, it would be nice to have an extensibility point around different ways of encoding data. Currently this spec is using an ABI encoding as specified in ARC4. However it might be more flexible to allow describing types independently from their encoding, and then also have an encoding field which can be used to reference an encoding spec, initially this might just be ARC4 ABI, but could allow other specifications down the track

To make sure I am understanding you correctly, you are basically advocating for type aliases. Like I can have
{ "timestamp": "uint64" }

Or something else?

Not type aliases, but a way of defining the encoding scheme used for arguments and results (which may also impact the valid types)
So at the moment this spec assumes an ARC4 ABI encoding is used, but if there was a field to explicitly specify what encoding is used then it would allow some flexibility in describing other apps that do not use ARC4 ABI

So at the moment this spec assumes an ARC4 ABI encoding is used, but if there was a field to explicitly specify what encoding is used then it would allow some flexibility in describing other apps that do not use ARC4 ABI

I feel like if another encoding is used then it should be a different standard that extends ARC56. Not using ABI encoding breaks downstream clients. Unless you are thinking the method signature and arg encoding would still be ABI-compliant, but there could be additional encoding information conveyed to the client.

I have updated the ARC and addressed some of the outstanding changes

ARC28 events now added
Added sourceInfo for source mapping and errors
Added optional defaultValue for method args
Allow network names, but only if genesis ID is also provided

The main outstanding points of discussion are

non-ARC4 encoding
static fields for things like fees

I think different encodings could be handled by creating a new ARC to extend this one. Right now there is no alternative encoding, so if clients were to support a new encoding they would also be able to support a new ARC.
Static fields is a can a worms. It is worth discussing, but I don't think it should hold up this ARC. Additional ARCs solving these problems can be created in the future.

@robdmoore Unless you object I will add you as an author to this ARC

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

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

So there's potentially three different kinds of states 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).

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.

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.

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

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

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

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

Starting to write code to convert ARC-32 t ARC-56 on the fly so we can have app clients that support both, but operate as ARC-56 primarily.
Question: Did we discuss the default argument options in ARC-32?
/**
 * Defines a strategy for obtaining a default value for a given ABI arg.
 */
export type DefaultArgument =
  | {
      /**
       * The default value should be fetched by invoking an ABI method
       */
      source: 'abi-method'
      data: ABIMethodParams
    }
  | {
      /**
       * The default value should be fetched from global state
       */
      source: 'global-state'
      /**
       * The key of the state variable
       */
      data: string
    }
  | {
      /**
       * The default value should be fetched from the local state of the sender user
       */
      source: 'local-state'
      /**
       * The key of the state variable
       */
      data: string
    }
  | {
      /**
       * The default value is a constant.
       */
      source: 'constant'
      /**
       * The static default value to use.
       */
      data: string | number
    }

The ARC-56 equivalent right now is:
    /** The default value that clients should use. MUST be base64 encoded bytes */
    defaultValue?: string

OK, here should be agood reflection of the two-way differences between ARC-32 and ARC-56:
export function arc32ToArc56(appSpec: AppSpec): Arc56Contract {
  const arc32Structs = Object.values(appSpec.hints).flatMap((hint) => Object.entries(hint.structs ?? {}))
  const structs = Object.fromEntries(
    arc32Structs.map(([name, struct]) => {
      const fields = Object.fromEntries(struct.elements.map((e) => [e[0], e[1]]))
      return [name, fields satisfies StructFields]
    }),
  ) satisfies { [structName: string]: StructFields }
  const hint = (m: ABIMethodParams) => appSpec.hints[new ABIMethod(m).getSignature()] as Hint | undefined
  const actions = (m: ABIMethodParams, type: 'CREATE' | 'CALL') => {
    const actions: Arc56Method['actions']['call'] = []
    const h = hint(m)
    if (h?.call_config.close_out && ['ALL', type].includes(h?.call_config.close_out)) actions.push('CloseOut')
    if (h?.call_config.delete_application && ['ALL', type].includes(h?.call_config.delete_application)) actions.push('DeleteApplication')
    if (h?.call_config.no_op && ['ALL', type].includes(h?.call_config.no_op)) actions.push('NoOp')
    if (h?.call_config.opt_in && ['ALL', type].includes(h?.call_config.opt_in)) actions.push('OptIn')
    if (h?.call_config.update_application && ['ALL', type].includes(h?.call_config.update_application)) actions.push('UpdateApplication')
    return actions
  }
  const bareActions = (type: 'CREATE' | 'CALL') => {
    const actions: Arc56Contract['bareActions']['call'] = []
    const b = appSpec.bare_call_config
    if (b.close_out && ['ALL', type].includes(b.close_out)) actions.push('CloseOut')
    if (b.delete_application && ['ALL', type].includes(b.delete_application)) actions.push('DeleteApplication')
    if (b.no_op && ['ALL', type].includes(b.no_op)) actions.push('NoOp')
    if (b.opt_in && ['ALL', type].includes(b.opt_in)) actions.push('OptIn')
    if (b.update_application && ['ALL', type].includes(b.update_application)) actions.push('UpdateApplication')
    return actions
  }

  return {
    arcs: [],
    name: appSpec.contract.name,
    desc: appSpec.contract.desc,
    structs: structs,
    methods: appSpec.contract.methods.map(
      (m) =>
        ({
          name: m.name,
          desc: m.desc,
          args: m.args.map((a) => ({
            name: a.name,
            type: a.type,
            desc: a.desc,
            struct: a.name ? hint(m)?.structs?.[a.name]?.name : undefined,
            //todo - no real equivocation: defaultValue: a.name ? hint(m)?.default_arguments?.[a.name]/* ? */ : undefined,
          })),
          returns: {
            type: m.returns.type,
            desc: m.returns.desc,
            struct: hint(m)?.structs?.output?.name,
          },
          events: [],
          readonly: hint(m)?.read_only,
          actions: {
            create: actions(m, 'CREATE') as unknown as Arc56Method['actions']['create'],
            call: actions(m, 'CALL'),
          },
        }) satisfies Arc56Method,
    ),
    state: {
      schema: {
        global: {
          ints: appSpec.state.global.num_uints,
          bytes: appSpec.state.global.num_byte_slices,
        },
        local: {
          ints: appSpec.state.local.num_uints,
          bytes: appSpec.state.local.num_byte_slices,
        },
      },
      keys: {
        global: Object.fromEntries(
          // todo: appSpec.schema.global/local.reserved not used - is that ok?
          Object.entries(appSpec.schema.global.declared).map((s) => [
            s[0],
            {
              key: Buffer.from(s[1].key, 'utf-8').toString('base64'),
              keyType: 'bytes',
              valueType: s[1].type,
              desc: s[1].descr,
              // todo: s[1].static not used - is that ok?
            } satisfies StorageKey,
          ]),
        ),
        local: Object.fromEntries(
          Object.entries(appSpec.schema.local.declared).map((s) => [
            s[0],
            {
              key: Buffer.from(s[1].key, 'utf-8').toString('base64'),
              keyType: 'bytes',
              valueType: s[1].type,
              desc: s[1].descr,
            } satisfies StorageKey,
          ]),
        ),
        box: {},
      },
      maps: {
        global: {},
        local: {},
        box: {},
      },
    },
    source: appSpec.source,
    bareActions: {
      create: bareActions('CREATE') as unknown as Arc56Contract['bareActions']['create'],
      call: bareActions('CALL'),
    },
    byteCode: undefined,
    compilerInfo: undefined,
    events: undefined,
    networks: undefined,
    scratchVariables: undefined,
    sourceInfo: undefined,
    templateVariables: undefined,
  } satisfies Arc56Contract
}

Question: Did we discuss the default argument options in ARC-32?

My thinking is that it will get kinda confusing if we support some ways of defining default values like global/local/box state but not others like method calls. From the HLL perspective, it's much easier to say that default arguments can only be constant values rather than having to explain why some values can be used and others can't.

My thinking is that it will get kinda confusing if we support some ways of defining default values like global/local/box state but not others like method calls.

Method calls was one of the ones originally supported.

From the HLL perspective, it's much easier to say that default arguments can only be constant values rather than having to explain why some values can be used and others can't.

Agreed, it's simpler, but it's extremely limiting, pretty much to the point of not being worth including at all. It also means that even if we start with constant values only there's no extensibility in this spec for possible future functionality (although I'd suggest it would be good to at least support what ARC-32 had so we don't have a regression in functionality).

Method calls was one of the ones originally supported.

Right but it's only partially supported which is part of the problem. There's no way to define arguments to said method call for example. We could try to support everything, but we end up writing a whole schema for defining default arguments which can get ugly real quick.

It also means that even if we start with constant values only there's no extensibility in this spec for possible future functionality

But this is part of the problem. Supporting all the ways of defining a default value will be a never ending cycle. We could easily end up in a scenario where a client only support X ways of defining default values but the ARC56 for a given app defines Y ways of default values and you have an incorgruency between the intended experience and clients.
If a developer wants more complex functionality, they can use special values (ie. if (assetId === MAX_UINT64) doWhatever()

(although I'd suggest it would be good to at least support what ARC-32 had so we don't have a regression in functionality).

I think this is a seldomly used feature so I don't think it's a huge deal if we don't support it.

There's no way to define arguments to said method call for example.

I think that's a reasonable limitation, you can create an argumentless method. And if in the future someone wants to create a more advanced ARC for more advanced use cases they would be able to since the schema here would be extendable.

But this is part of the problem. Supporting all the ways of defining a default value will be a never ending cycle. We could easily end up in a scenario where a client only support X ways of defining default values but the ARC56 for a given app defines Y ways of default values and you have an incorgruency between the intended experience and clients.

That's OK though, this is for default values, if you don't support a default value you force the user to specify it so no harm no foul.

I think this is a seldomly used feature so I don't think it's a huge deal if we don't support it.

I do think that's fair, but my recommendation is we still allow for extensibility and today we only define constant values, but allow for the type of value to be something else i.e. we use an object rather than a string.

I do think that's fair, but my recommendation is we still allow for extensibility and today we only define constant values, but allow for the type of value to be something else i.e. we use an object rather than a string.

Yeah fair. Thinking about it I supposed it's not really a worse experience of having some partial support for methods vs not supprting them at all for example. That was my main concern, the HLL experience. I don't think it's possible to have a perfect solution so we might as well support some ways of defining them.
How does the following sound for ways for defining default values

constant
box key
local state key (for sender)
global state key
0-arg method

If a developer tries to define a default value that is not supported we can throw an error list what is supported.

After implementing it, I'm comfortable with the key for global storage being base64, although per my other comment I think we still need a way to define a keyType for non-ARC4 string bytes.
Similarly, for a constant default value, I wonder if it's possible to have a non-ARC4 value, but maybe not since the parameter value would always be ARC-4??

keyType is defined as
 keyType: ABIType | AVMBytes | StructName

With AVMBytes = "bytes". So byte[] is for ABI byte array and bytes is for non-ABI byteslice

Similarly, for a constant default value, I wonder if it's possible to have a non-ARC4 value, but maybe not since the parameter value would always be ARC-4??

Yeah I think only ABI makes the most sense.

If you wanted to show a byte array as a string because it's AVMbytes, but it's known to be UTF-8 then I think that's useful.

The comments made by @robdmoore and @daniel-makerx has reminded me of one thing I have done in TEALScript implementation that has not been reflected in this ARC yet.
For every template variable, the value in TEAL is always an argument to pusbytes, even uint64. This is done primarily because the AVM assembly process does int compression, which means the PC mapping will vary depending on the value actually use. To get around this, TEALScript now does
pushbytes TMPL_MY_VAR
btoi
store X

By using scratch space we also save the repeated bytes which can inflate program size.

This is done primarily because the AVM assembly process does int compression,

By this do you mean the varuint encoding?

which means the PC mapping will vary depending on the value actually use. To get around this, TEALScript now does
pushbytes TMPL_MY_VAR
btoi
store X

If I understand this approach correctly, this solves the varying PC for uint64 template values as they are always encoded as 8 bytes with this approach?
However wouldn't there still be variable PC values for byte[] template values, as the value could be anywhere from 0 to 4096 bytes in length?

Correct, arbitrary bytes templates will vary in length but at compile time 'examples' could be provided to use in PC tracking?  It's quite possible template use may have a fixed length anyway but on some (likely) encoded value.

I'm keen for this standard to get finalised ASAP if we can so there isn't a moving target any more and we can finalise the algokit utils changes.
I just read through all remaining open threads of conversations and here is the summarised version:

Default arguments - add support for global state, local state, and static abi call per ARC-32 and also box key while we are at it
Moving networks jsdoc up to networks otherwise you can't see it in intellisense
My point about having AVMstring as well as AVMBytes we can take or leave it, in the end the implementation I did was if something is bytes you can pass in a string or a uint8array and if it's a string it just converts utf-8 to bytes so I think that's fine, if we want to provide the type generator a hint it would always be passed in as a string to remove the union type though maybe it's worth adding
Change jsdoc descriptions for the arg[i].type and return.type to indicate that  you should look at struct to see if the type represents a struct type
Change struct fields to be an ordered array
Allow template variable values to optionally be included in the network as metadata
Template variables - should we have a way of determining if we want uint64 that isn't encoded as bytes? I don't think ARC-56 should require uint's are always encoded as 64-bit bytes, it seems very restrictive and byte length will change anyway with any dynamic arrays
Be less restrictive about where template variables can be so it's OK to put them in constant blocks (leave out the regex for implementers to figure out - we will have reference implementations in utils we can link to?)

Agreed on all points but still want to resolve how to reconcile source mapping with template vars (uint64 and dynamic byteslices).

leave out the regex for implementers to figure out - we will have reference implementations in utils we can link to?

I think something with sufficient complexity in an ARC like this should be accompanied by a reference implementation, so I wouldn't want to finalize it until we have one that is reasonably tested.
Otherwise I wil update the ARC itself on all the resolved points tomorrow.

Assuming we put all TMPL values in the constant block, then the PC values of the following ops will not be variable if they are relative to the PC after constant blocks have been evaluated.
The sourceInfo would also needs to include which line/op the remaining PC values are offset from. This is an optional value that defaults to 0 which means all PC values are absolute
In the app-client-deprecation branch it would look something like this
const pcOffset = !appSpec.sourceInfo.pcLineOffset ? 0 : approvalSourceMap!.lineToPc[appSpec.sourceInfo.pcLineOffset][0]
const errorMessage = (isClearStateProgram ? appSpec.sourceInfo?.clear : appSpec.sourceInfo?.approval)?.find((s) =>
  (s?.pc?.map(o => o + pcOffset)?.includes(errorDetails?.pc ?? -1),
)?.errorMessage

@robdmoore I've implemented all points above.
Note that I've also changed the value "bytes" to "AVMBytes" and added "AVMString"
@daniel-makerx Seems reaonable to me. I would like to have some testing before we finalize. One thing to also consider is how we should handle programs that might not have a bytecblock or intcblock.

I've implemented all the above changes plus the pc mapping in TEALScript.
I realized the intcblock values are always 8 bytes, so that solves the int template variable problem. For dynamic byteslices I added a bytecblockPcOffset field that is an offset from the last PC for the bytecblock. You can see an example here: https://github.com/algorandfoundation/TEALScript/blob/3560faf4ade3990d67bd60556685460de47acddd/tests/contracts/artifacts/Templates.arc56_draft.json#L93
Full PR for the changes is here: https://github.com/algorandfoundation/TEALScript/pull/541
Before updating the ARC and finalizing I want to do some testing, but looks pretty good so far. The client will need to get the last PC associated with the bytecblock. I think that this should be fairly easy to parse entirely within the client if we agree to make bytecblock the first opcode in a program, otherwise the client will need to decompile the TEAL and determine the PC offset for the bytecblock.

I realized I was incorrect, intcblock values are not always 8 bytes and are uvarint encoded. Regardless, here is a reference implementation for how a client can parse the bytecode and get the constant block offsets. I'd still want to do a proper E2E test to make sure everything works as expected, but seems to work so far. This does, however, require that intcblock and/or bytecblock are the first opcodes in the program. I assume Puya already does this?
function parseConstantBlocks(bytes: number[]) {
    const programSize = bytes.length
    bytes.shift() // remove version
    const offsets: {bytecblock?: number, intcblock?: number} = { }

    while (bytes.length > 0) {
        const byte = bytes.shift()!

        if (byte === bytecblock || byte === intcblock) {
            const isBytecblock = byte === bytecblock
            const valuesRemaining = bytes.shift()!

            for (let i = 0; i < valuesRemaining; i++) {
                if (isBytecblock) {
                    // byte is the length of the next element
                    bytes.splice(0, bytes.shift()!)
                } else {
                    // intcblock is a uvarint, so we need to keep reading until we find the end (MSB is not set)
                    while ((bytes.shift()! & 0x80) !== 0) {}
                }
            }

            offsets[isBytecblock ? 'bytecblock' : 'intcblock'] = programSize - bytes.length

            if (bytes[0] !== bytecblock && bytes[0] !== intcblock) {
                // if the next opcode isn't a constant block, we're done
                break
            }
        }
    }

    return offsets
}

I've implemented this in TEALScript and have a E2E test here using the above function: https://github.com/algorandfoundation/TEALScript/blob/feat/cblocks/tests/templates.test.ts#L60
Seems to be working as expected. Unless there are any objections I can add cblocksPcOffset to the ARC and we can move to last call.

That approach is a little different than what we were thinking. Mostly in that the offset is implicit based on the constant blocks and requires inspection of the program bytes.
Below are the relevant changes we had in mind.
Key difference is including pcOffsetOpIndex as part of the sourceInfo, this explicitly describes which op the other PC's are relative to. And can be used in conjunction with the source map to determine the pcOffset.
Have not changed SourceInfo (aside from comments) but maybe pc could become relativePc to make it clearer?
interface Contract {
  ...
  sourceInfo?: {
    /** Approval program information */
    approval: ProgramSourceInfo;
    /** Clear program information */
    clear: ProgramSourceInfo;
  };
}

interface ProgramSourceInfo {
  /**
  When defined describes the the index of the op that all subsequent pc's are relative to
  can be used to ensure consistent pc mappings even when variable sized template values are used.
  Treat as 0 if not defined

  Note: #pragma version should be considered as an op for for this index (as it impacts the pc values and algod source maps)
  **/
  pcOffsetOpIndex?: number
  /* Program source information */
  lines: SourceInfo[]
}

interface SourceInfo {
  /** The line of pre-compiled TEAL */
  teal?: number;
  /**
  The program counter offset(s) that correspond to this line of TEAL
  If pcLineOffset is defined, pc values are relative to the pc of this op see `getAdjustedPcs` as an example
  **/
  pc?: Array<number>;
  /** A human-readable string that describes the error when the program fails at this given line of TEAL */
  errorMessage?: string;
  /** The line of the dissasembled TEAL this line of pre-compiled TEAL corresponds to */
  disassembledTeal?: number;
}

function getAdjustedPcs(programInfo: ProgramSourceInfo, sourceMap: algosdk.SourceMap): SourceInfo[] {
  // assuming one op per line, this obtains the pc value of the op at programInfo.pcOffsetOpIndex
  // note: lineToPc will include an entry for the program version if present, so pcOffsetOpIndex should reflect this
  const pcOffset = Object.values(sourceMap.lineToPc)[programInfo.pcOffsetOpIndex ?? 0][0]
  // remap pc value including offset
  return programInfo.lines.map((info) => {
    return {
      ...info,
      pc: info.pc?.map((p) => p + pcOffset),
    }
  })
}

Regarding your other question

This does, however, require that intcblock and/or bytecblock are the first opcodes in the program. I assume Puya already does this?

Yes Puya insert the constant blocks as the first two ops in the program (after #pragma version)

Also I am assuming that since we are going with the template variables in constant block approach we will update templateVariables.type to include AVMUInt64 before last call?

Key difference is including pcOffsetOpIndex as part of the sourceInfo,

I personally lean a bit more towards having a different key in SourceInfo becuase it prevents any implementers not accounting for the offset. Anecdotally, most contracts don't use template variables so it could be a gotcha for implementers. Now at least if an implementation doesn't properly support offsets it will just outright fail rather than give incorrect information.

And can be used in conjunction with the source map to determine the pcOffset

But with your approach you'd still need to inspect the bytes and parse potentially more opcodes than just bytecblock or intcblock (or get the dissasemlbed TEAL and respective source map but that is additional network calls and relies on disassemlby remaning constant) which seems more complex.

But with your approach you'd still need to inspect the bytes and parse potentially more opcodes

Don't need to parse the bytecode, just uses the algod source map to determine the correct offset. Which the new app-client already expects

or get the dissasemlbed TEAL and respective source map but that is additional network calls and relies on disassemlby remaning constant) which seems more complex.

I guess it depends on what the minimum information we are expecting to be available to implementations, I had assumed at a minimum that the TEAL would be available (and thus compiling it would provide the source maps). However I now realize this is not 100% clear.
For example when ARC-23 is used there is no source or bytecode provided.
Do we expect the pinned IPFS data in this case to provide 1 or both of those? (Note: ARC-23 says the TEAL MAY be provided, not MUST) We should probably document the expectations here?
And what about when ARC-23 is not used, which of these can be expected to be defined?

ARC-23 is not in use anywhere so I think we don't need to be limited by it necessarily (i.e. we can make changes to it).

ARC-23 or equivalent support in AlgoKit is on the list for next calendar year, until then there is no first class support and noone is using it to my knowledge.

I guess it depends on what the minimum information we are expecting to be available to implementations, I had assumed at a minimum that the TEAL would be available (and thus compiling it would provide the source maps). However I now realize this is not 100% clear.

I don't think we should assume that TEAL is always available. There are a few scenarios I can think of:

The publisher wants the TEAL to be closed source but still provide ARC56 for interaction
A client could interact with any number of deployed instances that have varying deploy-time variables that might not necessarily be known by the client.

Is ARC-23 the only reason the source field is optional? Would it make sense to tackle ARC-23 support separately?

Yes this is why these fields are optional. In the future we could add an additional field for source - ARC23 hash, but if we don't make it optional now it will be a breaking change.

If any 'client' code requires that the full source code of a contract be available just to call it, parse its return values, and to display friendly error messages for it - I would consider that a hard fail in the toolkits/clients.

We're all in agreement that TEAL is optional.

Had a brainstorm with @daniel-makerx and we came up with this suggestion:
// pc values are absolute, no need to offset
{
  sourceInfo: { approval: { mappings: { pc: ... }[], /* Optional: none is the default */ pcOffset: { offset: 'none'} } }
}
// pc values are offset to a teal line (naturally would only work if TEAL is provided in the ARC-56, but saves having to look up byte code from on-chain)
{
  sourceInfo: { approval: { mappings: { pc: ... }[], pcOffset: { offset: 'teal', value: 3 } } }
}
// pc values are offset to the cblocks, so you need to read TEAL source
{
  sourceInfo: { approval: { mappings: { pc: ... }[], pcOffset: { offset: 'cBlock' } } }
}

The reason this is useful is the compiler can tell the client the best way to resolve the PC values i.e. "don't bother" vs "get your hands on the byte code and calculate" vs "you have the teal and can calculate using the source map".

@robdmoore @pbennett @joe-p we are working (with @nullun) on two ARCs for AVM runtime errors, aimed at AVM compilers. Currently the ecosystem has conformed to the convention (unitl now not specified in any ARC) of using the combination of TEAL comments and SourceMap to return meaningful runtime errors from the AVM.
This first convention, regarding TEAL, is now being formalized in ARC-64, because different languages/compilers should follow it in the same way to ensure a homogeneous approach to AVM runtime errors.
That said, we are working on a second ARC (still wip), which allows AVM runtime errors "in-program", so directly from bytecode (not from TEAL).
Compilers/languages, in my opinion, should offer 2 options for compiling AVM runtime errors: one based on TEAL + SourceMap (ARC-64 for TEAL, ARC-56 for client) and the other purely based on program bytecode (which avoids publishing TEAL and the companion SourceMap artifact).

Makes sense @cusma. One comment: ARC-56 supports giving error messages against program counter and with the above suggestion would allow it to work without needing TEAL.

but saves having to look up byte code from on-chain)

The problem is it doesn't save from having to look up bytecode. Not only do you need to get the bytecode, but you also need to get the source map for the decompiled bytecode to properly calculate the PC offset.
Yes cblock offsets require bytecode but the alternatives are
A) Knowing deploy-time variables and having the pre-compiled TEAL
B) Decompiling the bytecode and comparing PC offsets to the provided source map
A is not guranteed and B seems more complex to me and relies on dissaembly never changing. It's not clear to me what the downside of cblock offsets are relative to the other options.

Yes we need the sourcemap, but in many circumstances it will already have that - the app client handles this for you. Admittedly, there are circumstances where you won't have sourcemap.
Maybe a better approach is to simply support both options? i.e. if you don't have source map then it can automatically grab the byte code and figure out based on cblock offset? Plus if there's a way of specifying that the values aren't offset they are absolute then it saves that extra call.
Maybe the structure then should be:
sourceInfo: {
  approval: {
    pcOffset: 'cBlock' | 'none',
    tealCBlockOffset: 3,
    mappings: [...]
  }
}

Nice thing is that also gives future extensibility for other pcOffset values in the future?

Nice thing is that also gives future extensibility for other pcOffset values in the future?

This is why I think just having a completely seperate key in each SourceInfo is nice. Clients can look for pc or cblocksPcOffset. In the future we can just use a different key name for different methods if we want to implement them. Clients that don't support this new method will simply not resolve errors, but nothing will crash and burn.

I do like putting pcOffset at the top level though so it's easy to discover which method is being used

Exactly, the thing is the same treatment will apply to all items so it makes more sense to have a key at the top level than change the key at the lower levels.
Then, for optimisation purposes it should be possible (default even?) to indicate there is no offset and they are absolute vs it's a cBlock offset.
The worst case of a client not supporting the offset type isn't a huge deal it just means the debugging info will be off, but if the mechanism to specify the type of offset exists on day 1 clients can read that value and act base don it (including opting out of debugging if it's a value they don't understand). I think that's way cleaner and less confusing than needing to read individual items and look for different keys. It's more efficient and more explicit.
On reflection, we also probably don't need the teal line offset since we can calculate that from source map if we have it and if we don't have it we can calculate it from the program bytes!

i.e.
sourceInfo: {
  approval: {
    pcOffset: 'cBlock' | 'none',
    mappings: [{pc: 4, ...}, ...]
  }
}

On reflection, we also probably don't need the teal line offset since we can calculate that from source map if we have it and if we don't have it we can calculate it from the program bytes!
Unfortunately source map on it's own isn't enough to know which line has the constant blocks as it only has line -> pc mappings

So if we are going to have the tealOffset option then that would need to be in the spec. But if we are going to support parsing bytecode anyway for the cOffset option maybe that is enough for now? It is explicit which is the key thing I wanted to see in the spec.

Ahh yeah that was my mistake.
I think it would be good to have an optional tealOffset key if the pcOffset is cBlock to avoid the extra byte code lookup, but it's not a hill I'd die on so comfortable if we don't have it.

@robdmoore @joe-p here you can find the ARC about AVM run time error in program bytecode I was mentioning in previous comments: https://github.com/algorandfoundation/ARCs/pull/315 (cc: @nullun)

@daniel-makerx @robdmoore In 35a19b3 I added ProgramSourceInfo interface that includes a pcOffsetMethod key.

I think it would be good to have an optional tealOffset key if the pcOffset is cBlock to avoid the extra byte code lookup, but it's not a hill I'd die on so comfortable if we don't have it.

Depending on consistency in decompiled output makes me a bit nervous. I think it's unlikely to happen but I also think it's relatively easy to parse the cblocks if they are at the top of the program.

In 19c6269 I removed teal from SourceInfo because we agreed it was only useful for developers reading the file. I realized, however, that the TEAL line numbers are also used to give better error messages even without an explicit error message, so I've added it back as an optional field RECOMMENDED for development, along with a sourec string


refl doesn't handle inductive values as expected in a certain case

Prerequisites

[X] Put an X between the brackets on this line if you have done all of the following:

Checked that your issue isn't already filed.

Specifically, check out the wishlist, open RFCs,
or feature requests.

Reduced the issue to a self-contained, reproducible test case.

Description
It would be natural to expect refl to work in the following case, but actually it doesn't.
Steps to Reproduce
Run the following code:
inductive a : Type
| foo : a → a
| bar : list a → a

definition f : a → a
| (a.foo x)         := a.foo x
| (a.bar [])        := a.bar []
| (a.bar (x :: xs)) := a.bar (f x :: xs)

example : ∀ x, f (a.foo x) = (a.foo x) :=
begin
  intros,
  refl
end

Expected behavior: [What you expect to happen]
No errors
Actual behavior: [What actually happens]
Lean detects an error at the refl, saying "invalid apply tactic, failed to unify f (a.foo x) = a.foo x with ?m_2 = ?m_2".
Reproduces how often: [What percentage of the time does it reproduce?]
100%
Versions
Lean (version 3.2.0, Release), macOS 10.12.6
Additional Information
The code passes type checking if you replace a.bar (f x :: xs) with a.bar (x :: xs).

This happens because f is defined via well-founded recursion, and then the equations for f don't hold definitionally.  In particular, f (foo x) = foo x is not true definitionally.  You can unfold f (foo x) manually using simp [f] or unfold f.

Thank you for your answer. Now I partially understand what is happening here.
But I still have an issue: unfold f doesn't change the goal in this case. Suppose I write the following code after the definition of a and f:
example : ∀ x, f (a.foo x) = (a.foo x) :=
begin
  intros,
  unfold f, -- (no effect)
  simp [f]
end

Though I can prove the equation using simp [f] (thank you!), I at the same time notice that the line unfold f does not change the goal from f (a.foo x) = a.foo x. I cannot find the reason.
Also, now I have to ask another question (I seem to have simplified the example too much). In the following code, even simp [f] doesn't work:
instance : has_inv a := has_inv.mk f

example : ∀ x, (a.foo x)⁻¹ = (a.foo x) :=
begin
  intros,
  -- simp [f] -- doesn't work
  -- refl     -- doesn't work
  unfold f,   -- (no effect)
  admit
end

I vaguely suspect that this is something related to reducible, but again I find it difficult to work out. Could you please guide me?

For me, unfold f works in this example.  So it will work in the next release.
For the inverse type class, you need to unfold has_inv.inv first:
example : ∀ x, (a.foo x)⁻¹ = (a.foo x) :=
by intros; unfold has_inv.inv f

example : ∀ x, (a.foo x)⁻¹ = (a.foo x) :=
by intros; simp [has_inv.inv, f]

Thank you for your kindness. I have got the point.
By the way, I should have tried the example using the latest commit. I'm sorry to bother you. I will be careful next time.


Omnis language support

Support for the Omnis programming language, used by Omnis Studio.

We're currently in a large discussion about how to handle language submissions:
https://github.com/highlightjs/highlight.js/issues/2149
Currently the maintainers don't have the time to review or maintain any new languages and related issues - so we've been recommending that they be published as separate packages/repos. To see what that looks like:
https://github.com/highlightjs/highlightjs-robots-txt
Just a heads up.

Thanks very much for the pointer, @yyyc514.
That is an interesting read, and I feel for the position you maintainers are in.
I am happy to wait until a decision is made on how best to handle new languages - good luck with that!

@egor-rogov I would certainly be willing to maintain a repo for the Omnis language.
@yyyc514 I'm very sorry, but I really don't have enough free time at the moment (and probably not the expertise) to be able to commit to helping maintain a "community" repo.
I can put time into maintaining the Omnis language, as I work on the Omnis development team, so I can do that on 'work time'.
I'm happy whether that's in a separate repo, or a community repo (I just wouldn't be able to put much time into other languages).
For our particular use-case, it's nice to be able to build a single JS package with our language embedded (which I guess would be more likely with the community repo?), but happy to go with whatever the official technique becomes.
The main thing for us is just having a central place where the language definition can be found and contributed to.

I can put time into maintaining the Omnis language, as I work on the Omnis development team, so I can do that on 'work time'.

Nice.

For our particular use-case, it's nice to be able to build a single JS package with our language embedded (which I guess would be more likely with the community repo?)

Not sure likely is the right word. I want it to be easy no matter where the cards fall on structure of repositories. Just we aren’t there yet.
It’s not particularly hard now - for a single language - just very manual.

Not sure likely is the right word. I want it to be easy no matter where the cards fall on structure of repositories. Just we aren’t there yet.
It’s not particularly hard now - for a single language - just very manual.

I agree - the process currently works well, it just needs a clever solution for adding languages in a maintainable way.

@jasongiss
https://github.com/highlightjs/highlight.js/issues/2328

Closing this as discussion can continue on #2328.


Problems with adminconsent endpoint

Hello,
I have two issues.
Issue 1
The first is an issue with the error response of the admin consent endpoint.
I'm using the adminconsent endpoint from https://docs.microsoft.com/en-us/azure/active-directory/develop/v2-permissions-and-consent#request-the-permissions-from-a-directory-admin that has a structure like
GET https://login.microsoftonline.com/{tenant}/adminconsent?
client_id=6731de76-14a6-49ae-97bc-6eba6914391e
&state=12345
&redirect_uri=http://localhost/myapp/permissions

This works fine for the success response. In the error response, instead of the expected error and error_description query params, I'm seeing the following which contains error, error_subcode and state.
https://github.localhost/orgs/team-sync/azure-callback?error=access_denied&error_subcode=cancel&state=<removed>

I'd like to have the tenant returned in the error response as well as reconciliation between what should be expected for the query params.
Issue 2
In subsequent adminconsent documentation, it is indicated to use a different endpoint (note the v2.0 in the URL):
// Line breaks are for legibility only.
	GET https://login.microsoftonline.com/{tenant}/v2.0/adminconsent?
  client_id=6731de76-14a6-49ae-97bc-6eba6914391e
  &state=12345
  &redirect_uri=http://localhost/myapp/permissions
	&scope=
	https://graph.microsoft.com/calendars.read
	https://graph.microsoft.com/mail.send

This endpoint expects a different set of query params provided to it but even after updating my code to use the new endpoint with the v2.0 included and the additional query params, I'm receiving the same response callback:
https://github.localhost/orgs/team-sync/azure-callback?error=access_denied&error_subcode=cancel&state=<removed>

Document Details
⚠ Do not edit this section. It is required for docs.microsoft.com ➟ GitHub issue linking.

ID: a5837e27-7af3-12ad-3fd7-10995017581e
Version Independent ID: 6ccb62ba-b31c-0cd5-c169-06c3ca899a4b
Content: Microsoft identity platform admin consent protocols
Content Source: articles/active-directory/develop/v2-admin-consent.md
Service: active-directory
Sub-service: develop
GitHub Login: @rwike77
Microsoft Alias: ryanwi

@wallace
Thanks for your feedback! We will investigate and update as appropriate.

@wallace, The response from adminconsent endpoint is always posted to the redirect_uri specified in the request. Based on the request that you have shared, any success or error response from  "https://login.microsoftonline.com/{tenant}/adminconsent" endpoint, should be posted to http://localhost/myapp/permissions and if there is any error thrown by consent framework, it would include error and error_description. This is applicable in case of both V1 and V2 endpoints.
However, in your case the redirect_uri parameter in the request is http://localhost/myapp/permissions and the error is posted at https://github.localhost/orgs/team-sync/azure-callback.
The reason why you are not getting the error and error_description could be because the error is being thrown by the application and not the adminconsent endpoint. As the response to below request, regardless of whether it is success or failure, should always be posted to http://localhost/myapp/permissions.
GET https://login.microsoftonline.com/{tenant}/adminconsent?
client_id=6731de76-14a6-49ae-97bc-6eba6914391e
&state=12345
&redirect_uri=http://localhost/myapp/permissions

I tested the same in my tenant, where I registered an application, configured http://localhost/myapp/permissions as redirect_uri (reply url) for the applicatoin and tested by making below call:
GET https://login.microsoftonline.com/my_tenant.onmicrosoft.com/adminconsent?
client_id=c55505d7-xxxx-xxxx-883e-e1c9e2544061
&state=12345
&redirect_uri=http://localhost/myapp/permissions

On the consent prompt, I clicked on cancel to generate an error:

I confirmed that the error is posted at the redirect_uri (http://localhost/myapp/permissions), which include both error and error_description. Below is the error message that I received:
http://localhost/myapp/permissions?error=access_denied&error_description=AADSTS65004%3a+The+resource+owner+or+authorization+server+denied+the+request.%0d%0aTrace+ID%3a+9d6037f1-xxxx-xxxx-xxxx-e0cf244b1100%0d%0aCorrelation+ID%3a+68e950a5-xxxx-xxxx-xxxx-0e01def76273%0d%0aTimestamp%3a+2019-10-23+08%3a45%3a02Z&admin_consent=True&tenant=312b290f-xxxx-xxxx-xxxx-db735079059f&state=12345
Suggestion: Please engage your application developer to modify the application's code for better exception handling and to include error and error_description if exception is encountered, as the error seems to be thrown by the application and not by Azure AD.
I hope this answers your queries. If you need any further assistance feel free to reach out to us at
<EMAIL_ADDRESS>

@amanmcse , I apologize for misleading you.
In my original message, I did not include the requests made in my case. Instead, I copied the requests from the documentation. I will now include my original requests to demonstrate that the problem exists.
My original request copied below:
https://login.microsoftonline.com/f71848b0-3319-4eb5-b0ab-df74225e725e/adminconsent?client_id=efcf6031-0d59-4349-9475-29cc3e77aa1b&state=AAAAAFWETPXMM2HMPLGRVXK5XNKQPAVPM5WG6YTBNRPXEZLMMF4V62LEXBGUIRLZJ5VTS6K2GJDHKYKYOBUGIR3MOZRGURJRVVYHE33WNFSGK4S7OR4XAZNHMF5HK4TFMFSA&redirect_uri=https%3A%2F%2Fgithub.localhost%2Forgs%2Fteam-sync%2Fazure-callback

A screenshot showing that request before I log the azure ad user who does not have admin consent capabilities:

Signing in the GHAdminOnly user takes me to this page:
https://login.microsoftonline.com/f71848b0-3319-4eb5-b0ab-df74225e725e/reprocess?ctx=rQIIAeNiUGC0UklNS04zMzA21DVIMbXUNTE2sdS1NDE31TWyTE42TjU3T0w0TCoS4hJYcIJJkStE0bVFzzVivcRaxVWMZhklJQXFVvr66ZklGaVJejn5yYk5GfnFJfr5RenF-iWpibm6xZV5yfqJVaVFqbpAyZykxOTsC4yMLxgZbzHxOKbkZuY55-cVp-aVzGJuYXQEAbdw15CACF9fIw_fAB_3oLAIb9MIP-_AAMewAF_TcHezyBAnv6CACNcoH19fN5MwMyMf1wgn91DPIJ8oL9OwkGAzbyN3LxcP70jvSH-nUHfPIGNf_6gg99Agr6CwsEgPV2PjcD-3YHdvk2Bz_yCTCMcoPw9fN1MPb5MQN1-3YMdNzCpp5oYWJhZJBrrGxobAAElNMtVNMkhM0k1JMzcxMjJNNQfiCyzCr1h4DFitODi4BPgk2BQYfrAwLmIFBpPwi-Nff3y-7bz_wq6v1zfPYjjFqq-d7JacHeCRZKRvGFpWaRoekFTkX1Bh4RsY6OJiWekbaZmrX5FV5mkeXmlga2plOIFN4RQbwwc2xg52hl2cZAYyAA2&sessionid=e3fa2913-05ac-45e2-85dc-c5883c9a3f2c

Screen shot of the above URL:

The URL for the "Return to the application without granting consent" is
https://login.microsoftonline.com/f71848b0-3319-4eb5-b0ab-df74225e725e/reprocess?ctx=rQIIAeNiUGC0UklNS04zMzA21DVIMbXUNTE2sdS1NDE31TWyTE42TjU3T0w0TCoS4hJYcIJJkStE0bVFzzVivcRaxVWMZhklJQXFVvr66ZklGaVJejn5yYk5GfnFJfr5RenF-iWpibm6xZV5yfqJVaVFqbpAyZykxOTsC4yMLxgZbzHxOKbkZuY55-cVp-aVzGJuYXQEAbdw15CACF9fIw_fAB_3oLAIb9MIP-_AAMewAF_TcHezyBAnv6CACNcoH19fN5MwMyMf1wgn91DPIJ8oL9OwkGAzbyN3LxcP70jvSH-nUHfPIGNf_6gg99Agr6CwsEgPV2PjcD-3YHdvk2Bz_yCTCMcoPw9fN1MPb5MQN1-3YMdNzCpp5oYWJhZJBrrGxobAAElNMtVNMkhM0k1JMzcxMjJNNQfiCyzCr1h4DFitODi4BPgk2BQYfrAwLmIFBpPwi-Nff3y-7bz_wq6v1zfPYjjFqq-d7JacHeCRZKRvGFpWaRoekFTkX1Bh4RsY6OJiWekbaZmrX5FV5mkeXmlga2plOIFN4RQbwwc2xg52hl2cZAYyAA2&sessionid=e3fa2913-05ac-45e2-85dc-c5883c9a3f2c#

Screenshot with the link that I clicked highlight in the orange box

which results in my browser being redirected to:
https://github.localhost/orgs/team-sync/azure-callback?error=access_denied&error_subcode=cancel&state=AAAAAFWETPXMM2HMPLGRVXK5XNKQPAVPM5WG6YTBNRPXEZLMMF4V62LEXBGUIRLZJ5VTS6K2GJDHKYKYOBUGIR3MOZRGURJRVVYHE33WNFSGK4S7OR4XAZNHMF5HK4TFMFSA

which does not contain an error_description.
If I understand correctly, your screenshots show the application being installed in the Microsoft Azure Directory whereas mine are focused granting permissions to the application. Could that be the difference?

@wallace, Thanks for sharing the details.
This is by design. If a standard user cancels the consent by clicking on "Return to the application without granting consent", Azure AD returns "error=access_denied&error_subcode=cancel". However, when the Global Administrator clicks on Cancel button at the consent prompt, Azure AD returns "error=access_denied&error_description=AADSTS65004%3a+The+resource+owner+or+authorization+server+denied+the+request."
The error_subcode=cancel can be treated as AADSTS65004 error.
Is there any specific requirement for which you need error_description to be included in first case?

Hi @amanmcse ,
My ideal would be to include the tenant in the response for a standard user so I can provide that user with a more informative error message that suggests next steps. Is there a possibility that the tenant would be included in the response for a regular user who clicks cancel?
I can work with the error_subcode value.
Thank you for your response.

@wallace,
Currently there is no provisioning in Azure AD for customizing this error message to include specific  information. You can post your idea at Azure Feedback portal and our product team will be notified.
Hope you have answer to your question. Please let me know if you need any further information.

@amanmcse ,
Thank you, you've been very helpful. I've added the suggestion as you recommended. One may find it here: https://feedback.azure.com/forums/34192--general-feedback/suggestions/38914249-include-tenant-query-parameter-in-the-admin-consen.
Thank you again for all your help.


Show shard group ID on SHOW SHARDS

Let's include all useful diagnostic information with this important command.
> show shards
name: _internal
---------------
id      database        retention_policy        shard_group     start_time              end_time                expiry_time             owners
1       _internal       monitor                 1               2015-11-09T00:00:00Z    2015-11-10T00:00:00Z    2015-11-17T00:00:00Z    1
3       _internal       monitor                 3               2015-11-10T00:00:00Z    2015-11-11T00:00:00Z    2015-11-18T00:00:00Z    1

@corylanou @dgnorton

+1


Externally hosted (loaded/fetched) GLTF files break SvelteKit adapter-vercel (SSR) preview on page reload

ReferenceError: self is not defined
at GLTFParser.loadTextureImage
at GLTFParser.loadTexture
related to #60

As mentioned here -> #60, it seems that generally anything related to / open to loading externally hosted assets is critical to SSR. Not sure if there's maybe another / better / more correct solution for this, but his works:
LoadedGLTF.svelte:

Make the loading logic entry point browser dependent.


NullPointerException in DB mock when mock value was null

[ ] I have searched the issues of this repository and believe that this is not a duplicate.

Ⅰ. Issue Description
When set the mock value null, would cause some NullPointerException during the CI.
Ⅱ. Describe what happened
Ⅲ. Describe what you expected to happen
I will fix this Problem.
Ⅳ. How to reproduce it (as minimally and precisely as possible)

Build The CI

Ⅴ. Anything else we need to know?
Ⅵ. Environment:

JDK version :
OS :
Others:

@l81893521 Paste the error stack.

@l81893521 Paste the error stack.

ok


Some controls are not read correctly in settings UI with screen readers.

Issue Type: Bug
@isidorn

Insure you are running with NVDA version 20.2. Other versions will probably show the same behavior.
Install the java extention pac.
Open the settings UI.
Go to extentions -> java
Tab through until you hear "edit in settings.json"
NVDA does not announce what I am actually goint to edit in settings.json if I click the button.

VS Code version: Code - Insiders 1.50.0-insider (9e505675670d65138405321a60b0df4ddec28799, 2020-09-16T06:49:37.816Z)
OS version: Windows_NT x64 10.0.19041
Remote OS version: Linux x64 4.19.84-microsoft-standard

System Info

Item
Value

CPUs
Intel(R) Core(TM) i3-7100U CPU @ 2.40GHz (4 x 2400)

GPU Status
2d_canvas: enabledflash_3d: enabledflash_stage3d: enabledflash_stage3d_baseline: enabledgpu_compositing: enabledmultiple_raster_threads: enabled_onoop_rasterization: disabled_offopengl: enabled_onprotected_video_decode: enabledrasterization: enabledskia_renderer: disabled_off_okvideo_decode: enabledvulkan: disabled_offwebgl: enabledwebgl2: enabled

Load (avg)
undefined

Memory (System)
7.87GB (1.89GB free)

Process Argv
--crash-reporter-id 13f64f2b-f4fe-4a58-989d-cf59898848a6

Screen Reader
yes

VM
0%

Item
Value

Remote
Dev Container: Go

OS
Linux x64 4.19.84-microsoft-standard

CPUs
Intel(R) Core(TM) i3-7100U CPU @ 2.40GHz (4 x 2400)

Memory (System)
6.10GB (2.09GB free)

VM
0%

Extensions (12)

Extension
Author (truncated)
Version

vscode-lombok
Gab
1.0.1

javadoc-tools
mad
1.4.0

remote-containers
ms-
0.141.0

remote-wsl
ms-
0.44.5

java
red
0.67.0

sonarlint-vscode
Son
1.17.0

vscodeintellicode
Vis
1.2.10

vscode-java-debug
vsc
0.28.0

vscode-java-dependency
vsc
0.13.0

vscode-java-pack
vsc
0.10.0

vscode-java-test
vsc
0.24.2

vscode-maven
vsc
0.24.2

If I understand the issue correctly: Some complex settings we do not allow to edit in the settings editor, for that we have to poen the settings.json.
So the fix would be that the aria label of the "edit in settings.json" button also controls the setting name. So for example
"edit terminal.integrated.automationShell.linux in settings.json"

That is correct and the way you describe it is how I would expect it to
behave.
On Fri, Sep 18, 2020 at 3:53 AM Isidor Nikolic<EMAIL_ADDRESS>wrote:

If I understand the issue correctly: Some complex settings we do not allow
to edit in the settings editor, for that we have to poen the settings.json.
So the fix would be that the aria label of the "edit in settings.json"
button also controls the setting name. So for example
"edit terminal.integrated.automationShell.linux in settings.json"
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
https://github.com/microsoft/vscode/issues/106975#issuecomment-694716784,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AAA5FIEFOIBXXLHEU7OPU6TSGMGX7ANCNFSM4RRIUZJQ
.


added my profile

Merged via https://github.com/Cornell-CS5356-Fall2015/cs5356/commit/79632539ba32ddb07aaf52d0cd46faa13140fb2a, thank you.


Don't close shared instances of ExecutorService or EventLoopGroup

If the executor/eventloop are shared among multiple Redisson instances shutting down of this Redissons shouldn't close the shared executor/eventloop

Thanks for pointed out!


Segmentation fault (core dumped)

I'm on ubuntu 16.04 (64bit) running an rx480 with custom rom (and the master repo as of 12 hours or so ago) but got a segmentation fault after about 20 minutes i think. I started it with
./zcash-miner -G -genproclimit=2 -stratum="stratum+tcp://zec.suprnova.cc:2142" -user=username -password=password
Here are the last few print statements of that's of any help...
 I'll start running it again to see if it consistently seg faults again

zogminer does not support genproclimit=2 yet. You need to run two zogminer instances next to each other.

Ah, okay. So "unofficially" I need two terminals both running that command or was I doing it correctly (aside from the lack of support)?


pls build  it

I can't build it, everytime i get error with Eigen3
I can't pair my controller at last release

hey I've built the latest version. Get it here :)
https://1drv.ms/u/s!AoFt-K2eey7si0h6hk4byW7cck7d?e=2dw0PE


World commands?

i made a huge mistake... i made a world with multiverse and i tped to it. i changed the gamerule of randomTickSpeed a bit toooooo high... now everytime i try to join i automaticly crash the server.. and if i try to change the tickspeed inside the console it changes for the main world (stable) not the side world (unstable) pls help

Hi, the solution to this would be to use the command /mv gamerule as it allows for per world configuration with command usage (in the console as well). If you used the vanilla command to specify your gamerule, you can use a program to modify your level.dat file and modify your randomTickSpeed value.
Closing ticket.


LNK110:Unable open file "glfw.lib"

emm,i encounter this problem when i build icg solution. then i already installed glfw3.1.2 in path:c:/program Filles(x86)/GLFW , and i just find the glfw3.lib in subfolder(/lib) of the folder:

i search on net,and i only find glfw3.lib,and nowhere about glfw.lib,so i am dejected of this problem,I would appreciate it if someone konw it？
in addtion,i rename the glfw.lib to glfw3.lib in the attach dependece place in visual studio, and it can't work yet, i got many LNK2019 errors.

@luoxiali can you share the steps for dealing with this issue,:-)

@Myzhencai i have a little bit short memory。but if you can try this it may be work:
1.change the cmakeconfig file in folder path:C:\Program Files (x86)\GLFW\lib\cmake\glfw   (perhaps any other path you install the glfw,mine is here by defualt)
2.replace the follow lines
set_and_check(GLFW3_INCLUDE_DIR "${PACKAGE_PREFIX_DIR}/include")
set_and_check(GLFW3_LIBRARY_DIR "${PACKAGE_PREFIX_DIR}/lib")
to
set_and_check(GLFW3_INCLUDE_DIR "${PACKAGE_PREFIX_DIR}/../../../include")
set_and_check(GLFW3_LIBRARY_DIR "${PACKAGE_PREFIX_DIR}/../../../lib")
3.in addition, CMAKE SETTING,
SET THE glfw3_DIR:C:/Program Files (x86)/GLFW/lib/cmake/glfw
that's my glfw's setting of this problem.


Android: Allow Mouse Capture

Allow mouse capture in android
Tested with https://github.com/thebestnom/Android-Tester
Im not 100% sure about where to with capture, currently I just clicked the last place before the capture (you can see that when the button will be continue to click even after the capture in the test app)
I'll cherrypick to 3.2 once the PR is approved

@thebestnom For my own education, what does mouse capture allow you to accomplish?

Mouse capture in android term is the ability to make the mouse relative and not based on location and hide the cursor (for fps style controls for example)

After rebasing to master it stopped working, Im testing to see why

@m4gr3d Ok, I found a bug on gdscript interpreter on android (or something of that kind)

you can see here the code, on windows it works as intended, but you can see in the code image what happnes... that so weird

This is looking like this is working, after 3 branch and cursor shape (that I'm already have working) Ill try to repreduce the bug minimaly
Also custom shape from native to java is hard

@m4gr3d Ok, I found a bug on gdscript interpreter on android (or something of that kind)

you can see here the code, on windows it works as intended, but you can see in the code image what happnes... that so weird
trying to make minimal code to repreduce that

That's very strange! @vnen Any idea what may be happening here?

Fixed everything 👍

Thanks!

@m4gr3d  Oh no, I missed a horrible mistake, opening a new PR now


min amount per deposit

Please go the the Preview tab and select the appropriate sub-template:
Cherry picking from https://github.com/informalsystems/cosmos-sdk/tree/masa/backport-18146-min-amount-per-deposit (related PR)

Production code - for types fix, feat, and refactor.
Docs - for documentation changes.
Others - for changes that do not affect production code.

There are ALOT of linting issues, which I will slowly chip away at

I think the entire Cosmos SDK codebase was merged here by mistake, while the idea was to cherry pick the x/gov modifications. A hint is the over 2000 files part of the PR most of which are additions. Just pointing it out fyi before it becomes even harder to revert without losing work done

I think the entire Cosmos SDK codebase was merged here by mistake, while the idea was to cherry pick the x/gov modifications. A hint is the over 2000 files part of the PR most of which are additions. Just pointing it out fyi before it becomes even harder to revert without losing work done.
Should also rebase onto main since we switched over having x/gov own types

This was the minimal import of the cosmos-sdk that it took to cherry pick that commit. I know it's hard to believe but I started with the commit and simply kept importing packages/modules until I could build

This was the minimal import of the cosmos-sdk that it took to cherry pick that commit. I know it's hard to believe but I started with the commit and simply kept importing packages/modules until I could build

If you look at the PR https://github.com/cosmos/cosmos-sdk/pull/19312/files this is what it touches

and in reality the changes in proto for staking and the change in bank can be discarded, so the PR is focused on x/gov. I don't think you really have to merge the whole sdk into the atomone app. Only the x/gov forked module needs to be updated (and the e2e tests I believe)


Implement Auth on API

Issue or current state
API has no auth.
OAuth implemented on bravo_ui, which will have functionality merged into this project.
Resolved when
OAuth implemented on all points of data access.
Implementation is copacetic with Bravo Vue UI.
Reference
Proof of concept implementation: https://github.com/grosscol/scenery-bailiff

Closed  by f7edd323c1488bdc78fee181a9ff504e698b3644


Chore: merge main-stbr into main-stbr-tfm-5 to enable deployments

Introduction :pencil2:
We need to update main-stbr-tfm-5 with main-stbr

Codecov Report
All modified and coverable lines are covered by tests :white_check_mark:

:exclamation: No coverage uploaded for pull request base (main-stbr-tfm-5@9cb4aa2). Click here to learn what that means.

Additional details and impacted files
@@                Coverage Diff                @@
##             main-stbr-tfm-5   #2431   +/-   ##
=================================================
  Coverage                   ?   7.95%
=================================================
  Files                      ?     786
  Lines                      ?   18297
  Branches                   ?    2721
=================================================
  Hits                       ?    1456
  Misses                     ?   14717
  Partials                   ?    2124

Flag
Coverage Δ

unit-test
7.95% <0.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.


How to use examples

How should a new user find or make use of the example files?
I'd suggest adding a section to the documentation - how to load examples into the web tool, or how to run them through the command-line tool, and then what to expect or look for in the tool/output. What are the examples meant to illustrate?
What are the sources of these datasets? Are any of them synthetic or created just for this project? I'd suggest adding citations and references as appropriate.

How should a new user find or make use of the example files?
I'd suggest adding a section to the documentation - how to load examples into the web tool, or how to run them through the command-line tool, and then what to expect or look for in the tool/output. What are the examples meant to illustrate?
What are the sources of these datasets? Are any of them synthetic or created just for this project? I'd suggest adding citations and references as appropriate.

The article originally had 3000 words, where all the information that has been requested was duly cited, but it was requested that the article be reduced to 100 words, which greatly impaired the quality of the work.

Could some of those 3000 words be added to the README or documentation? That would still be useful for users of the tool, even if it's not in scope for the paper.

How should a new user find or make use of the example files?
I'd suggest adding a section to the documentation - how to load examples into the web tool, or how to run them through the command-line tool, and then what to expect or look for in the tool/output. What are the examples meant to illustrate?
What are the sources of these datasets? Are any of them synthetic or created just for this project? I'd suggest adding citations and references as appropriate.

USING THE SAMPLE FILES
They were created by the author and are available under the same license as the program. I noticed that there were some files created by students that were exclusive to their master's projects. I removed them and put a pointer about the example files in README.md.


[Search] Fix 10-01-Preview and 11-01 GA swaggers

Data Plane API - Pull Request
This PR is meant to fix an omission in the swagger, where Edm.Single was omitted. Unfortunately, the swagger validation didn't catch this omission despite the example schemas including Collection(Edm.Single).
It also fixes an incorrect parameter name: customWebApiParameters instead of customVectorizer Parameters
API Info: The Basics
Most of the information about your service should be captured in the issue that serves as your API Spec engagement record.

Link to API Spec engagement record issue:

Is this review for (select one):

[x] Public preview release
[x] GA release

Change Scope
This section will help us focus on the specific parts of your API that are new or have been modified. Please share a link to the design document for the new APIs, a link to the previous API Spec document (if applicable), and the root paths that have been updated.

Design Document:
Previous API Spec Doc:
Updated paths:

Viewing API changes
For convenient view of the API changes made by this PR, refer to the URLs provided in the table
in the Generated ApiView comment added to this PR. You can use ApiView to show API versions diff.
Suppressing failures
If one or multiple validation error/warning suppression(s) is detected in your PR, please follow the
Swagger-Suppression-Process
to get approval.
❔Got questions? Need additional info?? We are here to help!

 Contact us!
The Azure API Review Board is dedicated to helping you create amazing APIs. You can read about our mission and learn more about our process on our wiki.

💬 Teams Channel
💌 email

Click here for links to tools, specs, guidelines & other good stuff
Tooling

Open API validation tools were run on this PR. Go here to see how to fix errors
Spectral Linting
Open API Hub

Guidelines & Specifications

Azure REST API Guidelines
OpenAPI Style Guidelines
Azure Breaking Change Policy

Helpful Links

Azure DevTools Wiki

@JeffreyRichter , @anuchandy please assist with approve breaking change label. Email has also been sent.

When I look at the breaking changes, I see:

A definition (CustomVectorizerParameters) removed from 2023-10-01-Preview. Breaking changes are allowed but must be made in a NEW api-version.
Added enum values (like am ,bs, ...) to 2023-10-01-preview. Adding enum values without changing api-version is only allowed for a private preview. Is this a private preview?
Added enum values (like Edm.Single) to 2023-11-01. Adding new enum values MUST be done to a new api-version since this is not a preview version.
Adding a new optional property (customWebApiParameters) to a preview. This is allowed if 2023-10-01-preview is in private preview and not public preview. Is it in private preview?

Since these changes are just fixing the swagger to properly represent the service, I'm adding the Approved-BreakingChange label.  We're also ignoring the existing PoliCheck warnings because there's no way to suppress them.  This PR is ready to merge.