namada icon indicating copy to clipboard operation
namada copied to clipboard

Duplicate Internal entries in a proposal JSON are filtered out

Open Rigorously opened this issue 9 months ago • 7 comments

Duplicate entries with the same amount and target in a PGF proposal are filtered out.

Some Donor Drop donors messed up their transactions and were not registered correctly. This has been corrected manually, resulting in multiple allocations.

One donor: 6,777 NAM qualified 1,667 NAM correction 1 1,667 NAM correction 2

Balance after the proposal passed was 8,334 NAM instead of the expected 10,000 NAM, because correction 2 was filtered out.

      {
        "Internal": {
          "amount": "6667000000",
          "target": "tnam1qzkx2k6zxjcdxf5tz4ep55mj4ll2kq7wsyrd5k5u"
        }
      },
      {
        "Internal": {
          "amount": "1667000000",
          "target": "tnam1qzkx2k6zxjcdxf5tz4ep55mj4ll2kq7wsyrd5k5u"
        }
      },
      {
        "Internal": {
          "amount": "1667000000",
          "target": "tnam1qzkx2k6zxjcdxf5tz4ep55mj4ll2kq7wsyrd5k5u"
        }
      },

https://explorer75.org/namada-housefire/accounts/tnam1qzkx2k6zxjcdxf5tz4ep55mj4ll2kq7wsyrd5k5u

See also: https://discord.com/channels/833618405537218590/1316142427672809514/1339775312816509041

Rigorously avatar Feb 14 '25 15:02 Rigorously

Specification requires a HashSet, while an Array was supplied.

Image

 "data": {
    "continuous": [],
    "retro": [
      {
        "Internal": {
          "amount": "1000000000",
          "target": "tnam1qp4d57kez3s2py6e5z5mc4cxjqj3ytpzqqrm5jsc"
        }
      },
      {
        "Internal": {
          "amount": "10000000000",
          "target": "tnam1qzdnlmyj6ce7dle98pnjdtv4vth4a075hy08w7d3"
        }

Rigorously avatar Feb 14 '25 16:02 Rigorously

This is done on purpose, the protocol doesn't accept duplicated entries to avoid weird error related to ordering. Just merge all the entries with the same address to a single entry.

Fraccaman avatar Feb 14 '25 16:02 Fraccaman

This is done on purpose, the protocol doesn't accept duplicated entries to avoid weird error related to ordering. Just merge all the entries with the same address to a single entry.

This can be fixed on the processing side, letting Rust merge duplicate entries if this behavior is wanted (maybe with --force), or reject the proposal if not.

Or on the input side, adding a key to each Internal item manually, but then the parser needs to be changed too.

The current method of silently ignoring duplicates is the worst, as the proposer might never have noticed.

Rigorously avatar Feb 14 '25 16:02 Rigorously

Result of brainstorming with @zenodeapp: using target tnam address as key for the Internal object forces the proposer to merge entries.

Rigorously avatar Feb 14 '25 16:02 Rigorously

we can validate and reject on the client side sure

Fraccaman avatar Feb 14 '25 18:02 Fraccaman

I'd be in favor of some kind of validation to enforce that there is only one value per provided Address. I say this without having looked at the code specifically yet, but seems easy. If not, then the tx should fail with error imo. It should really be on the proposer to construct a good proposal whose data can reliably be human-readable and simply digested imo as well.

brentstone avatar Feb 16 '25 05:02 brentstone

GM

toixa avatar Feb 16 '25 14:02 toixa