aiken-lang / aiken

A modern smart contract platform for Cardano
https://aiken-lang.org
Apache License 2.0
486 stars 98 forks source link

Inconsistent Decoding of Transaction Outputs in Post-Alonzo (Conway) Era #1017

Closed ilap closed 2 months ago

ilap commented 2 months ago

What Git revision are you using?

I'm using the latest commit as of the time of this issue: be31a7ce38a16bc3f3dad14b43535c58d4ef3f98

What operating system are you using, and which version?

Describe what the problem is?

The current implementation of the uplc crate in the Aiken project seems to me use the PreBabbage era transaction decoding method for the outputs, which expects an array for a TransactionOutput.

However, in the Post-Alonzo (Conway) era, a transaction output is either a map (preferred) or an array. This discrepancy is causing a decoding error when attempting to decode transaction outputs from the Post-Alonzo era using (an array/list of) map.

When trying to decode a Conway era CBOR transaction, the following error occurs:

unsupported era, please use Conway
Decoder error: unexpected type map at position 78: expected array

Note: This logic expects arrays, but Post-Alonzo transactions (including Conway) should use a map format for decoding UTXOs.

The example of a Conway era CBOR encoded transaction that triggers this error:

84aa0082825820666214b4886652876695f01a2dc20f75d90c9cae4d9e9cfd2a8b61b95c0c9d20038258200000000000000000000000000000000000000000000000000000000000000000020184a300581d703bcae3a853293f4343ce514dd1d6c3c3e53c4561ca84db8bcc5f699b011a0cfe6a80028201d818582bd8799f581cc8c47610a36034aac6fc58848bdae5c278d994ff502c05455e3b3ee81b00000191dd2c2ae8ff82583900c8c47610a36034aac6fc58848bdae5c278d994ff502c05455e3b3ee8f8ed3a0eea0ef835ffa7bbfcde55f7fe9d2cc5d55ea62cecb42bab3c821a00115cb0a1581c75c0c1c7643c07b42d89fdb1bf1c72594b650e1c03fc1d85eeae5437a1446164616d01a300581d70d4dc5145a60937eb4080545c64a39a48877e6a63f6ad3b4be946878001821a00151c56a1581c9c15e5d9d3a27077c32349ee644f3fa070d148bff17e25a4e8e37310a1416101028201d818584fd8799f01d87980446164616d413158206eceab8a03bbe884a91333138c9437c4b56068d9dd274a101006f58f5b9b907a581c75c0c1c7643c07b42d89fdb1bf1c72594b650e1c03fc1d85eeae5437ff82583900c8c47610a36034aac6fc58848bdae5c278d994ff502c05455e3b3ee8f8ed3a0eea0ef835ffa7bbfcde55f7fe9d2cc5d55ea62cecb42bab3c1b0000091841557c3a021a000a5ab90319018f082009a1581c75c0c1c7643c07b42d89fdb1bf1c72594b650e1c03fc1d85eeae5437a1446164616d010b5820550801e6529e51f30c53214ab6ce9c576082f90dce8b677404d50a93bbe0fe340d818258200000000000000000000000000000000000000000000000000000000000000000021082583900c8c47610a36034aac6fc58848bdae5c278d994ff502c05455e3b3ee8f8ed3a0eea0ef835ffa7bbfcde55f7fe9d2cc5d55ea62cecb42bab3c1b000009184e72a0001283825820666214b4886652876695f01a2dc20f75d90c9cae4d9e9cfd2a8b61b95c0c9d2000825820666214b4886652876695f01a2dc20f75d90c9cae4d9e9cfd2a8b61b95c0c9d2001825820666214b4886652876695f01a2dc20f75d90c9cae4d9e9cfd2a8b61b95c0c9d2002a2008182582000000000000000000000000000000000000000000000000000000000000000005840000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000582840001d87980821a00d59f801b00000002540be400840100d8799fd8799fd8799f413041604162ffd8799f413041604162ffd8799f5820400faa9e8294220de4e7a6ff7c42b21c1999815397c86cf4afa6095047515c0cd87a9f5820e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855ffd87a9f5820e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855ffffffff821a00d59f801b00000002540be400f5f6

It appears that the code segment responsible for decoding UTXOs is still expecting an array format. Specifically, this part of tx.rs: https://github.com/aiken-lang/aiken/blob/5879dcfd4c3e467559adc9a7d36e8942a2d8e5f7/crates/uplc/src/tx.rs#L115

for (input, output) in utxos_bytes {
    utxos.push(ResolvedInput {
        input: TransactionInput::decode_fragment(input)?,
        output: TransactionOutput::decode_fragment(output)?,
    });
}

What should be the expected behavior?

The TransactionOutput::decode_fragment method should be updated to handle both array and map structures, depending on the era of the transaction being decoded. This will ensure that transactions from both the PreBabbage and Post-Alonzo eras can be decoded correctly.

KtorZ commented 2 months ago

Good catch, thanks for the thorough report @ilap; will fix ASAP.

ilap commented 2 months ago

Hey @KtorZ,

Sorry, but it was my mistake. As I have just started to debug it and the error message the unexpected type map at position 78: expected array is the last (Alonzo's) of the three decodes when it cannot decode the tx for any era.

The issue that the in my tx the validity is set to -1 which should be an unsigned integer and the error cascades down to the alonzo decoding which expect an array.

So all good and sorry for any inconvenience caused by this.

    let multi_era_tx = MultiEraTx::decode_for_era(Era::Conway, tx_bytes)
        .or_else(|_| MultiEraTx::decode_for_era(Era::Babbage, tx_bytes))
        .or_else(|_| MultiEraTx::decode_for_era(Era::Alonzo, tx_bytes))?;